| Message ID | 20171206225450.92658-1-ffmpeg@tmm1.net |
|---|---|
| State | Accepted |
| Commit | 88e2dc7d0448d1d4656c78454bc5f17063b867e7 |
| Headers | show |
On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote: > From: Aman Gupta <aman@tmm1.net> > > --- > libavcodec/mpegvideo_parser.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c > index de70cd5632..be240b6890 100644 > --- a/libavcodec/mpegvideo_parser.c > +++ b/libavcodec/mpegvideo_parser.c > @@ -131,7 +131,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s, > } > } > > - if (!pc->progressive_sequence) { > + if (!pc->progressive_sequence && !progressive_frame) { Iam not against this if it results in more correct interlaced detection in practice But the spec uses progressive_sequence alone to determine the output ISO/IEC 13818-2: 1995 (E) / Recommendation ITU-T H.262 (1995 E) If progressive_sequence is 0 the period between two successive fields at the output of the decoding process is half of the reciprocal of the frame_rate. See Figure 7-20. So the commit message seems wrong or inprecisse [...]
On Thu, Dec 7, 2017 at 11:42 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote: > > From: Aman Gupta <aman@tmm1.net> > > > > --- > > libavcodec/mpegvideo_parser.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/mpegvideo_parser.c > b/libavcodec/mpegvideo_parser.c > > index de70cd5632..be240b6890 100644 > > --- a/libavcodec/mpegvideo_parser.c > > +++ b/libavcodec/mpegvideo_parser.c > > @@ -131,7 +131,7 @@ static void > mpegvideo_extract_headers(AVCodecParserContext *s, > > } > > } > > > > - if (!pc->progressive_sequence) { > > + if (!pc->progressive_sequence && > !progressive_frame) { > > Iam not against this if it results in more correct interlaced detection > in practice In my experience, most progressive mpeg2 samples are reported as interlaced by ffprobe before this change. See for example https://tmm1.s3.amazonaws.com/720p.ts > But the spec uses progressive_sequence alone to determine the output > > ISO/IEC 13818-2: 1995 (E) / Recommendation ITU-T H.262 (1995 E) > > If progressive_sequence is 0 the period between two successive fields > at the output of the decoding > process is half of the reciprocal of the frame_rate. See Figure 7-20. > > So the commit message seems wrong or inprecisse > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Rewriting code that is poorly written but fully understood is good. > Rewriting code that one doesnt understand is a sign that one is less smart > then the original author, trying to rewrite it will not make it better. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Thu, Dec 07, 2017 at 07:47:58PM +0000, Aman Gupta wrote: > On Thu, Dec 7, 2017 at 11:42 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote: > > > From: Aman Gupta <aman@tmm1.net> > > > > > > --- > > > libavcodec/mpegvideo_parser.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/mpegvideo_parser.c > > b/libavcodec/mpegvideo_parser.c > > > index de70cd5632..be240b6890 100644 > > > --- a/libavcodec/mpegvideo_parser.c > > > +++ b/libavcodec/mpegvideo_parser.c > > > @@ -131,7 +131,7 @@ static void > > mpegvideo_extract_headers(AVCodecParserContext *s, > > > } > > > } > > > > > > - if (!pc->progressive_sequence) { > > > + if (!pc->progressive_sequence && > > !progressive_frame) { > > > > Iam not against this if it results in more correct interlaced detection > > in practice > > > In my experience, most progressive mpeg2 samples are reported as interlaced > by ffprobe before this change. See for example > https://tmm1.s3.amazonaws.com/720p.ts ok, can you make the commit message convey this more clearly? as its now it looks like this is a fix for some spec non compliance [...]
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c index de70cd5632..be240b6890 100644 --- a/libavcodec/mpegvideo_parser.c +++ b/libavcodec/mpegvideo_parser.c @@ -131,7 +131,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s, } } - if (!pc->progressive_sequence) { + if (!pc->progressive_sequence && !progressive_frame) { if (top_field_first) s->field_order = AV_FIELD_TT; else
From: Aman Gupta <aman@tmm1.net> --- libavcodec/mpegvideo_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)