Return parsed sample frequency of ADTS header

Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests

Committed: https://crrev.com/3fe85a6016a2443502ec531c3f34ebb6dd6406a5
Cr-Commit-Position: refs/heads/master@{#399993}

[yucliu1, 2016-06-10 20:45:13]

Description was changed from

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device
==========

to

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device
==========

[yucliu1, 2016-06-10 20:45:13]

yucliu@chromium.org changed reviewers:
+ chcunningham@chromium.org, servolk@chromium.org

[servolk, 2016-06-10 22:15:07]

On 2016/06/10 20:45:21, yucliu1 wrote:

Can you please add a unit test that would check that doubled due to SBR sampling
rate is no longer clipped to 48KHz?

[yucliu1, 2016-06-13 19:11:00]

On 2016/06/10 22:15:07, servolk wrote:
> On 2016/06/10 20:45:21, yucliu1 wrote:
> 
> Can you please add a unit test that would check that doubled due to SBR
sampling
> rate is no longer clipped to 48KHz?

Add unit test to check parsed timestamp in buffers.

[yucliu1, 2016-06-13 19:17:38]

Description was changed from

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device
==========

to

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests
==========

[servolk, 2016-06-13 21:07:01]

On 2016/06/13 19:11:00, yucliu1 wrote:
> On 2016/06/10 22:15:07, servolk wrote:
> > On 2016/06/10 20:45:21, yucliu1 wrote:
> > 
> > Can you please add a unit test that would check that doubled due to SBR
> sampling
> > rate is no longer clipped to 48KHz?
> 
> Add unit test to check parsed timestamp in buffers.

lgtm

[chcunningham, 2016-06-13 23:34:06]

https://codereview.chromium.org/2063443002/diff/40001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/40001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: false);
IIUC, this false means you don't have SBR in your mime type, which means the
doubling code is not triggered

[yucliu1, 2016-06-14 00:06:10]

Rename force_timing to sbr_in_mimetype, since no test uses that boolean.

https://codereview.chromium.org/2063443002/diff/40001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/40001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: false);
On 2016/06/13 23:34:06, chcunningham wrote:
> IIUC, this false means you don't have SBR in your mime type, which means the
> doubling code is not triggered

No one use |force_timing|. So replace it with |sbr_in_mimetype|.

[chcunningham, 2016-06-14 00:13:07]

lgtm

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: return
ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
Missed one ;)

[yucliu1, 2016-06-14 00:23:28]

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: return
ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
On 2016/06/14 00:13:07, chcunningham wrote:
> Missed one ;)

Which one is missing?

[yucliu1, 2016-06-14 22:01:06]

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: return
ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
On 2016/06/14 00:13:07, chcunningham wrote:
> Missed one ;)

Ping~ I'm not sure what I'm missing here.

[chcunningham, 2016-06-14 23:27:41]

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: return
ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
On 2016/06/14 22:01:05, yucliu1 wrote:
> On 2016/06/14 00:13:07, chcunningham wrote:
> > Missed one ;)
> 
> Ping~ I'm not sure what I'm missing here.

sorry, I just mean that your comment says force_timing when it should be
sbr_in_mimetype

[yucliu1, 2016-06-14 23:42:25]

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
File media/formats/mp2t/es_parser_adts_unittest.cc (right):

https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
media/formats/mp2t/es_parser_adts_unittest.cc:45: return
ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
On 2016/06/14 23:27:41, chcunningham wrote:
> On 2016/06/14 22:01:05, yucliu1 wrote:
> > On 2016/06/14 00:13:07, chcunningham wrote:
> > > Missed one ;)
> > 
> > Ping~ I'm not sure what I'm missing here.
> 
> sorry, I just mean that your comment says force_timing when it should be
> sbr_in_mimetype

Ah, this one should be force_timing. It's used in es_parser_h264_unittests.

[chcunningham, 2016-06-15 18:28:52]

On 2016/06/14 23:42:25, yucliu1 wrote:
>
https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
> File media/formats/mp2t/es_parser_adts_unittest.cc (right):
> 
>
https://codereview.chromium.org/2063443002/diff/60001/media/formats/mp2t/es_p...
> media/formats/mp2t/es_parser_adts_unittest.cc:45: return
> ProcessPesPackets(&es_parser, pes_packets, false /* force_timing */);
> On 2016/06/14 23:27:41, chcunningham wrote:
> > On 2016/06/14 22:01:05, yucliu1 wrote:
> > > On 2016/06/14 00:13:07, chcunningham wrote:
> > > > Missed one ;)
> > > 
> > > Ping~ I'm not sure what I'm missing here.
> > 
> > sorry, I just mean that your comment says force_timing when it should be
> > sbr_in_mimetype
> 
> Ah, this one should be force_timing. It's used in es_parser_h264_unittests.

My bad. LGTM

[yucliu1, 2016-06-15 18:39:17]

The CQ bit was checked by yucliu@chromium.org

[yucliu1, 2016-06-15 18:39:18]

The patchset sent to the CQ was uploaded after l-g-t-m from servolk@chromium.org
Link to the patchset: https://codereview.chromium.org/2063443002/#ps60001
(title: "SBR")

[commit-bot: I haz the power, 2016-06-15 18:39:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2063443002/60001

[commit-bot: I haz the power, 2016-06-15 20:13:54]

Description was changed from

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests
==========

to

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests
==========

[commit-bot: I haz the power, 2016-06-15 20:13:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-15 20:13:57]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-15 20:15:19]

Description was changed from

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests
==========

to

==========
Return parsed sample frequency of ADTS header

In SBR mode, the sample frequency will be doubled. But it can't be
bigger than 48000. When calculating the frame duration, half the doubled
sample frequency won't get the original one.

BUG=internal b/29111813
TEST=Play stream on device, media_unittests

Committed: https://crrev.com/3fe85a6016a2443502ec531c3f34ebb6dd6406a5
Cr-Commit-Position: refs/heads/master@{#399993}
==========

[commit-bot: I haz the power, 2016-06-15 20:15:20]

Patchset 4 (id:??) landed as
https://crrev.com/3fe85a6016a2443502ec531c3f34ebb6dd6406a5
Cr-Commit-Position: refs/heads/master@{#399993}