Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
On Sun, Sep 27, 2015 at 10:11:47PM +0100, Thomas Mundt wrote: > IME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Default field order to top field first when interlaced frame is detected and > pic_struct_present_flag is not set. > Since bottom field first comes from the old NTSC standard and is not used > with HD anymore I think it??s straight forward to favor the majority. > > Signed-off-by: Thomas Mundt > --- > libavcodec/h264.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
IME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Default field order to top field first when interlaced frame is detected and
pic_struct_present_flag is not set.
Since bottom field first comes from the old NTSC standard and is not used with
HD anymore I think it´s straight forward to favor the majority.
Signed-off-by: Thomas Mundt
---
libavcodec/h264.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index b797893..8b95003 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -803,7 +803,7 @@ static void decode_postinit(H264Context *h, int
setup_finished)
/* Derive top_field_first from field pocs. */
cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
} else {
-if (cur->f->interlaced_frame || h->sps.pic_struct_present_flag) {
+if (h->sps.pic_struct_present_flag) {
/* Use picture timing SEI information. Even if it is a
* information of a past frame, better than nothing. */
if (h->sei_pic_struct == SEI_PIC_STRUCT_TOP_BOTTOM ||
@@ -811,6 +811,10 @@ static void decode_postinit(H264Context *h, int
setup_finished)
cur->f->top_field_first = 1;
else
cur->f->top_field_first = 0;
+} else if (cur->f->interlaced_frame) {
+/* Default to top field first when pic_struct_present_flag
+ * is not set but interlaced frame detected */
+cur->f->top_field_first = 1;
} else {
/* Most likely progressive */
cur->f->top_field_first = 0;
--
1.9.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
Seems yahoo emails become scrambled and links are not supported. Sorry! I´m on Win7 64Bit using latest media autobuild suite (msys2) from here:https://github.com/jb-alvarado/media-autobuild_suite I started fate with this command after rsync the testfiles:make fate SAMPLES="D:/ffmpeg/fate-suite/" fate-h264 passed without errors.The attached patch was unified diff. I thought this is okay.I´m using tortoise git which makes makes me crazy in combination with yahoo mail. But I ´ll try to send a git format-patch soon. Regards,Thomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
Michael Niedermayer schrieb am 18:04 Sonntag, 27.September 2015: >On Sun, Sep 27, 2015 at 03:24:48PM +, Thomas Mundt wrote: >> Fate always stops at vf_mergeplanes with error, with or without my >> patch:[AVFilterGraph @ 00eb4500] No such filter: '' >> Error initializing complex filters. >> Invalid argument >that should not happen, how can this be reproduced ? I´m on Win7 64Bit using latest media autobuild suite (msys2) from hereI started fate with this command after rsync the testfiles:make fate SAMPLES="D:/ffmpeg/fate-suite/">> Is there a way to limit fate that it just checks h264 part?>make fate-h264Thanks! fate-h264 passed without errors. >also please submit a git patch (which includes a commit message) >git format-patch if you lack teh setup for working send-emailI will do in a >separate email... Regards,Thomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
On Sun, Sep 27, 2015 at 03:24:48PM +, Thomas Mundt wrote: > Fate always stops at vf_mergeplanes with error, with or without my > patch:[AVFilterGraph @ 00eb4500] No such filter: '' > Error initializing complex filters. > Invalid argument that should not happen, how can this be reproduced ? > Is there a way to limit fate that it just checks h264 part? make fate-h264 also please submit a git patch (which includes a commit message) git format-patch if you lack teh setup for working send-email [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
Fate always stops at vf_mergeplanes with error, with or without my patch:[AVFilterGraph @ 00eb4500] No such filter: '' Error initializing complex filters. Invalid argument Is there a way to limit fate that it just checks h264 part? Kieran Kunhya schrieb am 13:23 Sonntag, 27.September 2015: On 27 September 2015 at 09:09, Thomas Mundt wrote: > Hi Kieran, > no, you´re right. I misinterpreted SEI_PIC_STRUCT_FRAME because it´s also the > default when pic_struct_present_flag is 0. Just checked avci header - no > pic_struct_present_flag. Sorry!But fixing avci field order detection should > be done.Does attached patch make more sense to you? > Regards,Thomas Hi, Yes patch makes sense assuming it doesn't break fate etc. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
On 27 September 2015 at 09:09, Thomas Mundt wrote: > Hi Kieran, > no, you´re right. I misinterpreted SEI_PIC_STRUCT_FRAME because it´s also the > default when pic_struct_present_flag is 0. Just checked avci header - no > pic_struct_present_flag. Sorry!But fixing avci field order detection should > be done.Does attached patch make more sense to you? > Regards,Thomas Hi, Yes patch makes sense assuming it doesn't break fate etc. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
Hi Kieran, no, you´re right. I misinterpreted SEI_PIC_STRUCT_FRAME because it´s also the default when pic_struct_present_flag is 0. Just checked avci header - no pic_struct_present_flag. Sorry!But fixing avci field order detection should be done.Does attached patch make more sense to you? Regards,Thomas Kieran Kunhya schrieb am 20:59 Samstag, 26.September 2015: On 26 September 2015 at 16:59, Thomas Mundt wrote: > Hi, > some h264 encoders, like broadcast avc-intra in this case avc-intra sets pic-struct. Is this new? Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel h264.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
On 26 September 2015 at 16:59, Thomas Mundt wrote: > Hi, > some h264 encoders, like broadcast avc-intra in this case avc-intra sets pic-struct. Is this new? Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]h264: Fix ticket #3147 H264 - Wrong field order
Hi, some h264 encoders, like broadcast avc-intra in this case, always set sei_pic_struct to "frame" even at interlaced encodings with separate fields. FFmpeg h264 decoder always assumes field order as "bottom field first" unless "top field first" is definitely detected. This leads to wrong field order detection in some cases.Attached patch changes this behavior of h264 decoder when sei_pic_struct is used for field order detection.Now "top field first" is assumed unless "bottom field first" is definitely detected. One could say fine, but now "bottom field first" material with sei_pic_struct set to "frame" will be detected wrong.Yes, but since "bottom field first" comes from the old NTSC standard and is not used with HD anymore I think it´s straight forward to favor the majority. Please comment,Thomas h264.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
