[2/2] movdec: Set frame_size for AMR

diff mbox

From: Carl Eugen Hoyos <cehoyos@ag.or.at>

The amr demuxer also sets the frame_size field.
---
 libavformat/mov.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

On 11/4/11 6:25 AM, Martin Storsjö wrote:
> From: Carl Eugen Hoyos<cehoyos@ag.or.at>
>
> The amr demuxer also sets the frame_size field.
> ---

I'd use a switch.

lu

On Fri, 4 Nov 2011, Luca Barbato wrote:

> On 11/4/11 6:25 AM, Martin Storsjö wrote:
>> From: Carl Eugen Hoyos<cehoyos@ag.or.at>
>> 
>> The amr demuxer also sets the frame_size field.
>> ---
>
> I'd use a switch.

This actually alread is within a larger switch, with two statements being 
common to the both AMR forms, while the sample rate and frame size differ. 
Having two nested switches for the same thing looks just weird IMO, and 
duplicating the two common statements feels needless.

// Martin

On 11/04/2011 09:25 AM, Martin Storsjö wrote:

> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> 
> The amr demuxer also sets the frame_size field.


it doesn't need to though. it already sets pkt->duration.

> ---
>  libavformat/mov.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2036f51..131e2b4 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1295,10 +1295,13 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries)
>          st->codec->frame_size= sc->samples_per_frame;
>          st->codec->channels= 1; /* really needed */
>          /* force sample rate for amr, stsd in 3gp does not store sample rate */
> -        if (st->codec->codec_id == CODEC_ID_AMR_NB)
> +        if (st->codec->codec_id == CODEC_ID_AMR_NB) {
>              st->codec->sample_rate = 8000;
> -        else if (st->codec->codec_id == CODEC_ID_AMR_WB)
> +            st->codec->frame_size  = 160;
> +        } else if (st->codec->codec_id == CODEC_ID_AMR_WB) {
>              st->codec->sample_rate = 16000;
> +            st->codec->frame_size  = 320;
> +        }
>          break;
>      case CODEC_ID_MP2:
>      case CODEC_ID_MP3:


On the input side, the only thing frame_size is used for is guessing
packet duration.  The mov/mp4 demuxer already has other ways of
determining correct packet duration.  I don't think this change actually
does anything.

-Justin

On Fri, 4 Nov 2011, Justin Ruggles wrote:

> On 11/04/2011 09:25 AM, Martin Storsjö wrote:
>
>> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>>
>> The amr demuxer also sets the frame_size field.
>
>
> it doesn't need to though. it already sets pkt->duration.
>
>> ---
>>  libavformat/mov.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 2036f51..131e2b4 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -1295,10 +1295,13 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries)
>>          st->codec->frame_size= sc->samples_per_frame;
>>          st->codec->channels= 1; /* really needed */
>>          /* force sample rate for amr, stsd in 3gp does not store sample rate */
>> -        if (st->codec->codec_id == CODEC_ID_AMR_NB)
>> +        if (st->codec->codec_id == CODEC_ID_AMR_NB) {
>>              st->codec->sample_rate = 8000;
>> -        else if (st->codec->codec_id == CODEC_ID_AMR_WB)
>> +            st->codec->frame_size  = 160;
>> +        } else if (st->codec->codec_id == CODEC_ID_AMR_WB) {
>>              st->codec->sample_rate = 16000;
>> +            st->codec->frame_size  = 320;
>> +        }
>>          break;
>>      case CODEC_ID_MP2:
>>      case CODEC_ID_MP3:
>
>
> On the input side, the only thing frame_size is used for is guessing
> packet duration.  The mov/mp4 demuxer already has other ways of
> determining correct packet duration.  I don't think this change actually
> does anything.

That might very well be the case, patch dropped.

// Martin