On 04/20/2011 03:28 PM, Ronald S. Bultje wrote:

> Hi,
> 
> 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.

Thanks,
Justin



>From 965521452c17adb7f357e46d11568dea18c3ed1b Mon Sep 17 00:00:00 2001
From: Justin Ruggles <[email protected]>
Date: Tue, 19 Apr 2011 18:50:20 -0400
Subject: [PATCH 1/5] Check AVCodec.supported_samplerates and AVCodec.channel_layouts in
 avcodec_open().
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.1"

This is a multi-part message in MIME format.
--------------1.7.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


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(-)


--------------1.7.1
Content-Type: text/x-patch; name="0001-Check-AVCodec.supported_samplerates-and-AVCodec.chan.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Check-AVCodec.supported_samplerates-and-AVCodec.chan.patch"

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 0190e66..4966aad 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -542,8 +542,9 @@ int attribute_align_arg avcodec_open(AVCodecContext *avctx, AVCodec *codec)
         ret = AVERROR(EINVAL);
         goto free_and_end;
     }
-    if (avctx->codec->sample_fmts && avctx->codec->encode) {
+    if (avctx->codec->encode) {
         int i;
+        if (avctx->codec->sample_fmts) {
         for (i = 0; avctx->codec->sample_fmts[i] != AV_SAMPLE_FMT_NONE; i++)
             if (avctx->sample_fmt == avctx->codec->sample_fmts[i])
                 break;
@@ -552,6 +553,31 @@ int attribute_align_arg avcodec_open(AVCodecContext *avctx, AVCodec *codec)
             ret = AVERROR(EINVAL);
             goto free_and_end;
         }
+        }
+        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");
+                ret = AVERROR(EINVAL);
+                goto free_and_end;
+            }
+        }
+        if (avctx->codec->channel_layouts) {
+            if (!avctx->channel_layout) {
+                av_log(avctx, AV_LOG_WARNING, "channel_layout not specified\n");
+            } else {
+                for (i = 0; avctx->codec->channel_layouts[i] != 0; i++)
+                    if (avctx->channel_layout == avctx->codec->channel_layouts[i])
+                        break;
+                if (avctx->codec->channel_layouts[i] == 0) {
+                    av_log(avctx, AV_LOG_ERROR, "Specified channel_layout is not supported\n");
+                    ret = AVERROR(EINVAL);
+                    goto free_and_end;
+                }
+            }
+        }
     }
 
     if(avctx->codec->init && !(avctx->active_thread_type&FF_THREAD_FRAME)){

--------------1.7.1--


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

Reply via email to