Hi,

On Wed, Apr 20, 2011 at 5:09 PM, Justin Ruggles
<[email protected]> wrote:
> On 04/20/2011 03:28 PM, Ronald S. Bultje wrote:
>> On Wed, Apr 20, 2011 at 3:25 PM, Justin Ruggles
>> <[email protected]> wrote:
>>> On 04/20/2011 03:14 PM, Ronald S. Bultje wrote:
>>>> On Tue, Apr 19, 2011 at 7:09 PM, Justin Ruggles
>>>> <[email protected]> wrote:
>>>>>
>>>>> If the encoder has a channel_layouts list and 
>>>>> AVCodecContext.channel_layout
>>>>> is 0, then only print a warning and let the encoder decide how to handle 
>>>>> it.
>>>>> ---
>>>>>  libavcodec/utils.c |   28 +++++++++++++++++++++++++++-
>>>>>  1 files changed, 27 insertions(+), 1 deletions(-)
>>>> [..]
>>>>> +        if (avctx->codec->supported_samplerates) {
>>>>> +            for (i = 0; avctx->codec->supported_samplerates[i] != 0; i++)
>>>>> +                if (avctx->sample_rate == 
>>>>> avctx->codec->supported_samplerates[i])
>>>>> +                    break;
>>>>> +            if (avctx->codec->supported_samplerates[i] == 0) {
>>>>> +                av_log(avctx, AV_LOG_ERROR, "Specified sample_rate is 
>>>>> not supported\n");
>>>>
>>>> Specify which samplerates _are_ supported, maybe?
>>>
>>> see below.
>>>
>>>>> +        if (avctx->codec->channel_layouts) {
>>>>
>>>> if .. && avctx->channel_layout? Or is that always set at this point?
>>>
>>> It is not always set, but the patch implements different behavior
>>> depending on if it is set or not.  If it is set, it requires a matching
>>> supported layout.  If it is not set, it prints a warning.
>>
>> Well, you're still looping. I'd do
>>
>> if (layout) {
>>   if (codec->layouts) {
>>     loop ;
>>     error if not found;
>>   }
>> } else {
>>   warn;
>> }
>>
>> This prevents running the unnecessary loop.
>
> new patch attached
>
>>>>> +                    av_log(avctx, AV_LOG_ERROR, "Specified 
>>>>> channel_layout is not supported\n");
>>>>
>>>> Please specify which are supported.
>>>
>>> This is just for apps that are using the API incorrectly. I don't think
>>> we need to go that far.
>>
>> So this code is never triggered when running a broken AVI file of
>> 12345Hz and trying to transcode it to ac3 using the ffmpeg tool? In
>> that case it's OK.
>
>
> the ffmpeg tool checks the supported sample rates list if one exists and
> chooses the one that is the closest to the source sample rate.  see
> ffmpeg.c:choose_sample_rate()
>
> actually ac3enc doesn't set supported_samplerates yet, but once it does
> the ffmpeg tool would choose 12000Hz for the encoder and do resampling
> from 12345Hz.

OK then (please also reindent the code that is now misindented).

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to