From patchwork Sat Nov 18 10:41:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "pkv.stream" X-Patchwork-Id: 6162 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.161.94 with SMTP id m30csp1682650jah; Sat, 18 Nov 2017 02:42:07 -0800 (PST) X-Google-Smtp-Source: AGs4zMZoJhrb40nhitQdUbUCzMpWhDTGQyW/La3s54YaScMQeaiH0wC3BRuKCMT8JebYGmIzOViu X-Received: by 10.223.145.230 with SMTP id 93mr7207220wri.190.1511001727299; Sat, 18 Nov 2017 02:42:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511001727; cv=none; d=google.com; s=arc-20160816; b=NVtbuP6vEKxVK1A4vIvuO0mY9Gchac7GA7BW1SEybjGAtcluJXKhznTUc3HBiHKvNy NRd/r9iXdiBH2/x851ZAoVu6iJP2aSB4pffNItan6M4RwZqvExV5fctJ1LhNapKKVV9C rJZsgpkiDrrPAfFoH44U5OHLTzn1ugzgpizIADU7IMm6kj3dM+a/KfKDTT3P32XkGGKL n1dfHDNYt8i9A1m+/o7yNP9wZIpVEJsO7i1uDaintJf+wGCS4dWAx6cTbScz4QLRmo1h SqTLYaG7bhoOEdCr1dQdaYOtBB5tB7y6SSE2X94OrRziCPYwMszus4N9sJuTVDfUZfgu zYrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:to:dkim-signature:delivered-to :arc-authentication-results; bh=MPgkWJsf3Z8cu3yzLEGzWFtpfrwJSMUjQ2dFTlEtKsc=; b=ZRroyXSNGI7jeCs1i7rCWlgneA0xzTNhYrsyjdUQe5pAARp9UzBy0qb6hx1bgficQA Zm1CIvjPsuPI9MUfGCV2D20MwyXScxdNmTaVTmOQFNUVxgdkn8zxEtBb3ieN3Btpm11L Q+MqVbOvsTHt6NlOXJ6EXuunR+xvVHRMqIZd/koI1UHUOANQNB1YZFEDRszw5nr4hdVO cXXmyG4nEJjy0iEvrzuKuuZvHZUtau6vOqvCdrM84PrYfjVDJtnBRpzWqewwp5bUS8C/ qmQ8YhOQ4ObiFQvdJ2hYHrqGFj7S+vR3JIasaQVLb7LVx2IfweqfNJyYmFaJMq6ofsr+ lCJA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=kQYmy6tx; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id e191si4313138wmg.208.2017.11.18.02.42.06; Sat, 18 Nov 2017 02:42:07 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=kQYmy6tx; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BAB94689AA9; Sat, 18 Nov 2017 12:41:48 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 03D336883DC for ; Sat, 18 Nov 2017 12:41:42 +0200 (EET) Received: by mail-wm0-f42.google.com with SMTP id b189so10729582wmd.0 for ; Sat, 18 Nov 2017 02:41:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language; bh=IZT9Xr5XTy0bEC9x2bn9nbMva0qJmziE6eqf5dRQ/fc=; b=kQYmy6txx7aojzv5znlJ0HigHF8zfO8iOKTcs5mPqDDEcTO62tGUtbJkr90S7izg/g 1eisqyPe/0Zm3BeJTQ6LZHkWgQcEEBETSiUaXuko4zdraZCs2GZHXqizPWSdy0jLCziJ rt235kO77m0YDeMsnKCOdSeF2h2lQwpNAeYaKj7A1HA0z2y309qWzpnmPFkaXHD8Nb5+ LuIpvUhHvAP9v6yJv1ve03yM6Hv2UgRdr3z0X3Ew/Vpj64FCDWOWQ6DfEJjTxjH3YZKz 4pDp7XJPya0wBYYDfWe+eR1bNZIxOrCD7yNL6AnTBMSWDaax8a/yYrfrUqCwl95hr9tb M/Kg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=IZT9Xr5XTy0bEC9x2bn9nbMva0qJmziE6eqf5dRQ/fc=; b=DnrvbsCZsiVaQ5M3WCNe99SBuTFKgIeGolkavJTf1TvjbTw7GGBH7e/JvVAjEnknlb 7viKtE5joVKNS1ArY3+0NsgG3qiQGDGOWrz5A2sW0yrATpPaeGu5khWeNp954xTY1H6u k/K+wBlFQWT90E617szYE/V2wTvc9Hp9q2zx3+SHdEN+3fVWMb3psMm/UZmHYU5JfOZE woTn76lrikRFBr7DQo4JATr2j0OaImKibA5aWXpJOz7boc3XcKow0u84o50BPF4sw2ko nNavRL24idXs8SuDSX3bwTzvmw4t5+m7pjrUje2j8YANocIVIClkLjn8HILooSUGseIi 7kAw== X-Gm-Message-State: AJaThX4+S9al951eqejeUOsiofg4lQgPkzHGgEUYyecSYqjLvNwUD0x2 W4aZjRptqTK20NLtKnzdwjrmqw== X-Received: by 10.80.226.74 with SMTP id o10mr11335834edl.290.1511001717393; Sat, 18 Nov 2017 02:41:57 -0800 (PST) Received: from [192.168.0.2] ([176.159.7.188]) by smtp.googlemail.com with ESMTPSA id j27sm4746668eda.59.2017.11.18.02.41.56 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 18 Nov 2017 02:41:56 -0800 (PST) To: ffmpeg-devel@ffmpeg.org References: <20171110001236.GX6009@nb4> <7ed2b80e-1b72-5461-0050-3656e042d3f9@gmail.com> <20171111000728.GZ6009@nb4> <6111ca51-c953-284a-9d57-905bf8b5fcf1@gmail.com> <20171112160048.GQ6009@nb4> <20171112163827.GT6009@nb4> <19c44254-1279-b685-3f46-0f03b2fa7a07@gmail.com> <20171114121355.GB6009@nb4> <16cfbba9-053d-0958-0042-6487d93301e9@gmail.com> <20171116014847.GT6009@nb4> From: "pkv.stream" Message-ID: <148ce8f6-476b-a8b1-a7ae-d44cd2e44316@gmail.com> Date: Sat, 18 Nov 2017 11:41:54 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171116014847.GT6009@nb4> Content-Language: fr X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [DEVEL][PATCH v3] ffmpeg: fix channel_layout bug on non-default layout X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Hi Michael >>>> ffmpeg_opt.c | 12 ++++++++++-- >>>> 1 file changed, 10 insertions(+), 2 deletions(-) >>>> a7c71fb1ccd7d91b61033be70dfd324b4e3f3c34 0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch >>>> From fb7f7f6e01cc242e13d0e640f583dffe6e7a8ada Mon Sep 17 00:00:00 2001 >>>> From: pkviet >>>> Date: Mon, 2 Oct 2017 11:14:31 +0200 >>>> Subject: [PATCH] ffmpeg: fix channel_layout bug on non-default layout >>>> >>>> Fix for ticket 6706. >>>> The -channel_layout option was not working when the channel layout was not >>>> a default one (ex: for 4 channels, quad was interpreted as 4.0 which is >>>> the default layout for 4 channels; or octagonal interpreted as 7.1). >>>> This led to the spurious auto-insertion of an auto-resampler filter >>>> remapping the channels even if input and output had identical channel >>>> layouts. >>>> The fix operates by directly calling the channel layout if defined in >>>> options. If the layout is undefined, the default layout is selected as >>>> before the fix. >>>> --- >>>> fftools/ffmpeg_opt.c | 12 ++++++++++-- >>>> 1 file changed, 10 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c >>>> index ca6f10d..8941d66 100644 >>>> --- a/fftools/ffmpeg_opt.c >>>> +++ b/fftools/ffmpeg_opt.c >>>> @@ -1785,6 +1785,7 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in >>>> AVStream *st; >>>> OutputStream *ost; >>>> AVCodecContext *audio_enc; >>>> + AVDictionaryEntry *output_layout; >>>> ost = new_output_stream(o, oc, AVMEDIA_TYPE_AUDIO, source_index); >>>> st = ost->st; >>>> @@ -1799,7 +1800,10 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in >>>> char *sample_fmt = NULL; >>>> MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st); >>>> - >>>> + output_layout = av_dict_get(ost->encoder_opts,"channel_layout", NULL, AV_DICT_IGNORE_SUFFIX); >>>> + if (output_layout) { >>>> + audio_enc->channel_layout = strtoull(output_layout->value, NULL, 10); >>>> + } >>> why is this handled different from audio_channels ? >>> that is why is this not using MATCH_PER_STREAM_OPT() ? >>> (yes this would require some changes but i wonder why this would be >>> handled differently) >> Hi >> I did try to use the MATCH_PER_STREAM_OPT() but didn't manage to >> have it working. Also I was a bit hesitant to modify the >> OptionsContext struct, and preferred something minimal. >> If you think this can definitely be made to work without too much >> coding and prefer such a solution, I'll retry working on a >> MATCH_PER_STREAM_OPT() solution. > i dont really know if it "can definitely be made to work without too much > coding", it just seemed odd how this is handled differently I have another version of the patch working with MATCH_PER_STREAM_OPT() ; but the changes to code are more important, and it's a bit hacky (defines an internal OptionDef) ... so not sure it is any better than the few lines of patch v3. I've checked that stream specifiers ( :a:0 ) are passed correctly and that two streams with different layouts are also treated correctly (the previous patch without MATCH_PER_STREAM_OPT() works also; those were two points you singled out in your review).  I have no real opinion on the best course, which to pick or even to let the bug hanging (I'm only trying to fix it due to my work on the aac PCE patch of atomnuker ; the bug prevents full use of the new PCE capability). It's Ok with me if you decide to forgo these attempts to fix the bug and let the bug stay. I'm not impacted by the bug in my case use (encode 16 channels in aac); just trying to be thorough in my (akward) contributions and trying to give back to the project. Tell me the best course; or if you see a way to make my MATCH_PER_STREAM_OPT() code less hacky. Regards > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 00c3c724544b16c19282b39644e2584f9c4a4181 Mon Sep 17 00:00:00 2001 From: pkviet Date: Sat, 18 Nov 2017 00:26:50 +0100 Subject: [PATCH] ffmpeg: fix ticket 6706 Fix regression with channel_layout option which is not passed correctly from output streams to filters when the channel layout is not a default one. --- fftools/cmdutils.h | 1 + fftools/ffmpeg.h | 3 +++ fftools/ffmpeg_opt.c | 41 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h index 2997ee3..fa2b255 100644 --- a/fftools/cmdutils.h +++ b/fftools/cmdutils.h @@ -155,6 +155,7 @@ typedef struct SpecifierOpt { uint8_t *str; int i; int64_t i64; + uint64_t ui64; float f; double dbl; } u; diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index e0977e1..5c45df4 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -121,6 +121,8 @@ typedef struct OptionsContext { int nb_frame_sizes; SpecifierOpt *frame_pix_fmts; int nb_frame_pix_fmts; + SpecifierOpt *channel_layouts; + int nb_channel_layouts; /* input options */ int64_t input_ts_offset; @@ -624,6 +626,7 @@ extern int vstats_version; extern const AVIOInterruptCB int_cb; extern const OptionDef options[]; +extern const OptionDef channel_layout_option[]; extern const HWAccel hwaccels[]; extern AVBufferRef *hw_device_ctx; #if CONFIG_QSV diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index 47d3841..ab614a8 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -1803,6 +1803,7 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in char *sample_fmt = NULL; MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st); + MATCH_PER_STREAM_OPT(channel_layouts, ui64, audio_enc->channel_layout, oc, st); MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st); if (sample_fmt && @@ -2527,7 +2528,11 @@ loop_end: (count + 1) * sizeof(*f->sample_rates)); } if (ost->enc_ctx->channels) { - f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels); + if (ost->enc_ctx->channel_layout) { + f->channel_layout = ost->enc_ctx->channel_layout; + } else { + f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels); + } } else if (ost->enc->channel_layouts) { count = 0; while (ost->enc->channel_layouts[count]) @@ -3104,8 +3109,9 @@ static int opt_channel_layout(void *optctx, const char *opt, const char *arg) char layout_str[32]; char *stream_str; char *ac_str; - int ret, channels, ac_str_size; + int ret, channels, ac_str_size, stream_str_size; uint64_t layout; + const char *spec = "a"; layout = av_get_channel_layout(arg); if (!layout) { @@ -3116,12 +3122,33 @@ static int opt_channel_layout(void *optctx, const char *opt, const char *arg) ret = opt_default_new(o, opt, layout_str); if (ret < 0) return ret; + /* factored code between 'ac' option and 'channel_layout_spec' option */ + stream_str = strchr(opt, ':'); + stream_str_size = (stream_str ? strlen(stream_str) : 0); + /* set 'channel_layout_spec' internal option which stores channel_layout + * (as uint64) in SpecifierOpt, which enables access to channel_layout through MATCH_PER_STREAM_OPT + */ + GROW_ARRAY(o->channel_layouts, o->nb_channel_layouts); + o->channel_layouts[o->nb_channel_layouts - 1].specifier = spec; + ac_str_size = 20 + stream_str_size; + ac_str = av_mallocz(ac_str_size); + if (!ac_str) { + return AVERROR(ENOMEM); + } + av_strlcpy(ac_str, "channel_layout_spec", 20); + if (stream_str) { + av_strlcat(ac_str, stream_str, ac_str_size); + } + ret = parse_option(o, ac_str, layout_str, channel_layout_option); + if (ret < 0) { + return ret; + } + av_free(ac_str); /* set 'ac' option based on channel layout */ channels = av_get_channel_layout_nb_channels(layout); snprintf(layout_str, sizeof(layout_str), "%d", channels); - stream_str = strchr(opt, ':'); - ac_str_size = 3 + (stream_str ? strlen(stream_str) : 0); + ac_str_size = 3 + stream_str_size; ac_str = av_mallocz(ac_str_size); if (!ac_str) return AVERROR(ENOMEM); @@ -3758,3 +3785,9 @@ const OptionDef options[] = { { NULL, }, }; +/* internal OptionDef used to enable storage of channel_layout option in a SpecifierOpt */ +const OptionDef channel_layout_option[] = { + { "channel_layout_spec", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC | + OPT_INPUT | OPT_OUTPUT,{ .off = OFFSET(channel_layouts) }, + "stores channel layout in SpecifierOpt ", "layout_hex" }, +};