Re: [libav-devel] [PATCH] avconv: fix guessed channel layout being lost during stream copy

2016-09-20 Thread Anton Khirnov
Quoting James Almer (2016-09-18 19:07:52)
> The guessed layout was being stored in the decoder context, which in the case 
> of
> stream copy is unused.
> 
> Signed-off-by: James Almer 
> ---
> Didn't run FATE after this patch, but aside from guess_input_channel_layout()
> nothing seemed to be using ist->dec_ctx inside the switch statement, so moving
> the avcodec_parameters_to_context() call down shouldn't break anything.
> 
> Unless i'm missing something, this may be the first case of an input codecpar
> being modified outside of the demuxer that filled it.
> I assume this is acceptable, otherwise the solution would probably be more
> complex or less clean.

I tend to disagree here -- the doxy in lavf says that when demuxing
codecpar is filled by lavf. I would interpret this as meaning that the
caller is not allowed to randomly modify it and the demuxer may rely on
the information in it staying the same.

This specific change probably won't break any actual demuxers, but as
avconv tends to be seen as the "reference API user", I'd prefer it to
use the API correctly. It shouldn't be much work to make a private copy
of the codecpar and modify that instead.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] avconv: fix guessed channel layout being lost during stream copy

2016-09-19 Thread Diego Biurrun
On Sun, Sep 18, 2016 at 02:07:52PM -0300, James Almer wrote:
> The guessed layout was being stored in the decoder context, which in the case 
> of
> stream copy is unused.
> 
> Signed-off-by: James Almer 
> ---
> Didn't run FATE after this patch, but aside from guess_input_channel_layout()
> nothing seemed to be using ist->dec_ctx inside the switch statement, so moving
> the avcodec_parameters_to_context() call down shouldn't break anything.

It does pass FATE. Please run FATE yourself next time.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


[libav-devel] [PATCH] avconv: fix guessed channel layout being lost during stream copy

2016-09-18 Thread James Almer
The guessed layout was being stored in the decoder context, which in the case of
stream copy is unused.

Signed-off-by: James Almer 
---
Didn't run FATE after this patch, but aside from guess_input_channel_layout()
nothing seemed to be using ist->dec_ctx inside the switch statement, so moving
the avcodec_parameters_to_context() call down shouldn't break anything.

Unless i'm missing something, this may be the first case of an input codecpar
being modified outside of the demuxer that filled it.
I assume this is acceptable, otherwise the solution would probably be more
complex or less clean.

 avconv.c |  2 +-
 avconv_opt.c | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/avconv.c b/avconv.c
index 59eb300..0390b47 100644
--- a/avconv.c
+++ b/avconv.c
@@ -1310,7 +1310,7 @@ static int decode(AVCodecContext *avctx, AVFrame *frame, 
int *got_frame, AVPacke
 
 int guess_input_channel_layout(InputStream *ist)
 {
-AVCodecContext *dec = ist->dec_ctx;
+AVCodecParameters *dec = ist->st->codecpar;
 
 if (!dec->channel_layout) {
 char layout_name[256];
diff --git a/avconv_opt.c b/avconv_opt.c
index 362a5b7..d9c2318 100644
--- a/avconv_opt.c
+++ b/avconv_opt.c
@@ -545,12 +545,6 @@ static void add_input_streams(OptionsContext *o, 
AVFormatContext *ic)
 exit_program(1);
 }
 
-ret = avcodec_parameters_to_context(ist->dec_ctx, par);
-if (ret < 0) {
-av_log(NULL, AV_LOG_ERROR, "Error initializing the decoder 
context.\n");
-exit_program(1);
-}
-
 switch (par->codec_type) {
 case AVMEDIA_TYPE_VIDEO:
 MATCH_PER_STREAM_OPT(frame_rates, str, framerate, ic, st);
@@ -621,6 +615,12 @@ static void add_input_streams(OptionsContext *o, 
AVFormatContext *ic)
 default:
 abort();
 }
+
+ret = avcodec_parameters_to_context(ist->dec_ctx, par);
+if (ret < 0) {
+av_log(NULL, AV_LOG_ERROR, "Error initializing the decoder 
context.\n");
+exit_program(1);
+}
 }
 }
 
-- 
2.9.1

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel