GNOME Bugzilla – Bug 722453
avviddec: add output-corrupt property
Last modified: 2014-01-17 21:14:55 UTC
Latest changes in libav decoder seem to have unset the codec context flag CODEC_FLAG_OUTPUT_CORRUPT. This means that GStreamer will never get corrupted frames from libav even if these frames have been correctly decoded. Since bug 722290, buffers are marked with GST_BUFFER_FLAG_CORRUPTED when receiving corrupted frames from libav. However, this will never be set because of the recent changes in libav.
Created attachment 266576 [details] [review] add output-corrupt property We set output-corrupt property to TRUE by default so we go back to the original GStreamer behavior.
Review of attachment 266576 [details] [review]: Almost good, but please make your commit message more similar to the other ones and: ::: ext/libav/gstavviddec.c @@ +47,3 @@ #define DEFAULT_DEBUG_MV FALSE #define DEFAULT_MAX_THREADS 0 +#define DEFAULT_OUTPUT_CORRUPT TRUE Please properly indent this @@ +1850,3 @@ + ffmpegdec->output_corrupt = g_value_get_boolean (value); + gst_ffmpegviddec_context_set_flags (ffmpegdec->context, + CODEC_FLAG_OUTPUT_CORRUPT, ffmpegdec->output_corrupt); You check inside the function if there is a context or not... and do that with an assertion. But here in set_property you might not have a context yet IIRC
Created attachment 266580 [details] [review] add output-corrupt property (fixup) Not sure if it makes sense to set/unset the flags every time, so I just update the output_corrupt property now and only set flags in gst_ffmpegviddec_open where the context is initialized for sure. The context is created in the init function, so I think old code was also fine. In any case, setting the flags before gst_ffmpeg_avcodec_open is called (which happens when setting property for the first time) doesn't work. About the indentation, it was because of my editor and the tabs. Is it OK to use tabs in the code?
Review of attachment 266580 [details] [review]: ::: ext/libav/gstavviddec.c @@ +47,3 @@ #define DEFAULT_DEBUG_MV FALSE #define DEFAULT_MAX_THREADS 0 +#define DEFAULT_OUTPUT_CORRUPT TRUE We use spaces instead of tabs, but I'll fix that :)
(In reply to comment #4) > Review of attachment 266580 [details] [review]: > > ::: ext/libav/gstavviddec.c > @@ +47,3 @@ > #define DEFAULT_DEBUG_MV FALSE > #define DEFAULT_MAX_THREADS 0 > +#define DEFAULT_OUTPUT_CORRUPT TRUE > > We use spaces instead of tabs, but I'll fix that :) Good to know, and better :-). I'll also fix it next time I see it.
Also please try to make commit messages in the future that are a bit closer to the others :) But thanks for the patch! commit 60f7b00db39cefd236ef199cde54d5d2d3b5ed2c Author: Aleix Conchillo Flaqué <aleix@oblong.com> Date: Fri Jan 17 12:38:23 2014 -0800 avviddec: Add output-corrupt property The output-corrupt property will set the CODEC_FLAG_OUTPUT_CORRUPT flag in the codec context. The user can now decide whether libav outputs corrupt frames or not. Previous libav versions had this flag always set. https://bugzilla.gnome.org/show_bug.cgi?id=722453
(In reply to comment #6) > Also please try to make commit messages in the future that are a bit closer to > the others :) But thanks for the patch! > Sure, sorry about that. Btw, this and the fix for bug 722290 work great now :-).