[1/2] avconv: Get the default channel layout before deciding whether to reinit resampling

diff mbox

For demuxers/decoders that reset the channel layout regularly
(e.g. rtpdec_qt) to a default value of 0, we avoid reinitializing
the resampling needlessly, since the call to
guess_input_channel_layout within the resample_changed block would
end up with the same channel layout anyway.

Does this make the call to guess_input_channel_layout totally
redundant, or is there a case where it still is necessary?

This fixes reception of
rtsp://streams.ymlmedia.com/streams.ymlmedia.com/YML070404s.mov.
---
 avconv.c |    2 ++
 1 file changed, 2 insertions(+)

On 09/07/2012 03:57 AM, Martin Storsjö wrote:
> For demuxers/decoders that reset the channel layout regularly
> (e.g. rtpdec_qt) to a default value of 0, we avoid reinitializing
> the resampling needlessly, since the call to
> guess_input_channel_layout within the resample_changed block would
> end up with the same channel layout anyway.
> 
> Does this make the call to guess_input_channel_layout totally
> redundant, or is there a case where it still is necessary?
> 
> This fixes reception of
> rtsp://streams.ymlmedia.com/streams.ymlmedia.com/YML070404s.mov.
> ---
>  avconv.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/avconv.c b/avconv.c
> index ea73606..e122ee0 100644
> --- a/avconv.c
> +++ b/avconv.c
> @@ -1150,6 +1150,8 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output)
>  
>      rate_emu_sleep(ist);
>  
> +    if (!decoded_frame->channel_layout)
> +        decoded_frame->channel_layout = av_get_default_channel_layout(avctx->channels);
>      resample_changed = ist->resample_sample_fmt     != decoded_frame->format         ||
>                         ist->resample_channels       != avctx->channels               ||
>                         ist->resample_channel_layout != decoded_frame->channel_layout ||

I think it still is good to have guess_input_channel_layout to notify
the user that avconv is guessing the layout.

Both patches look ok, but maybe they should also check if it was unable
to re-guess the layout since the channel count may have changed mid-stream.

-Justin

On Fri, 7 Sep 2012, Justin Ruggles wrote:

> On 09/07/2012 03:57 AM, Martin Storsjö wrote:
>> For demuxers/decoders that reset the channel layout regularly
>> (e.g. rtpdec_qt) to a default value of 0, we avoid reinitializing
>> the resampling needlessly, since the call to
>> guess_input_channel_layout within the resample_changed block would
>> end up with the same channel layout anyway.
>>
>> Does this make the call to guess_input_channel_layout totally
>> redundant, or is there a case where it still is necessary?
>>
>> This fixes reception of
>> rtsp://streams.ymlmedia.com/streams.ymlmedia.com/YML070404s.mov.
>> ---
>>  avconv.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/avconv.c b/avconv.c
>> index ea73606..e122ee0 100644
>> --- a/avconv.c
>> +++ b/avconv.c
>> @@ -1150,6 +1150,8 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output)
>>
>>      rate_emu_sleep(ist);
>>
>> +    if (!decoded_frame->channel_layout)
>> +        decoded_frame->channel_layout = av_get_default_channel_layout(avctx->channels);
>>      resample_changed = ist->resample_sample_fmt     != decoded_frame->format         ||
>>                         ist->resample_channels       != avctx->channels               ||
>>                         ist->resample_channel_layout != decoded_frame->channel_layout ||
>
> I think it still is good to have guess_input_channel_layout to notify
> the user that avconv is guessing the layout.

Yes... I'm a bit undecided about this one. Using 
guess_input_channel_layout here requires more changes, since it sets 
avctx->channel_layout while this checks decoded_frame->channel_layout. 
After the guess_input_channel_layout call below, 
decoded_frame->channel_layout is simply set to avctx->channel_layout.

> Both patches look ok, but maybe they should also check if it was unable
> to re-guess the layout since the channel count may have changed mid-stream.

Since I'm not too sure about these two myself, I'm in no hurry to apply 
them (especially since you were ok with the changes to mov_chan.c), so you 
could try to restructure this part in an even better way if you have the 
time, since you're more familiar with it than I am.

// Martin