Re: [FFmpeg-devel] [PATCH] libavcodec/mpegvideo_parser: set field_order correctly for progressive mpeg2
On Thu, Dec 07, 2017 at 07:47:58PM +, Aman Gupta wrote:
> On Thu, Dec 7, 2017 at 11:42 AM Michael Niedermayer
> wrote:
>
> > On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote:
> > > From: Aman Gupta
> > >
> > > ---
> > > 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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavcodec/mpegvideo_parser: set field_order correctly for progressive mpeg2
On Thu, Dec 7, 2017 at 11:42 AM Michael Niedermayer
wrote:
> On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote:
> > From: Aman Gupta
> >
> > ---
> > 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
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavcodec/mpegvideo_parser: set field_order correctly for progressive mpeg2
On Wed, Dec 06, 2017 at 02:54:50PM -0800, Aman Gupta wrote:
> From: Aman Gupta
>
> ---
> 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
[...]
--
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.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavcodec/mpegvideo_parser: set field_order correctly for progressive mpeg2
From: Aman Gupta
---
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) {
if (top_field_first)
s->field_order = AV_FIELD_TT;
else
--
2.14.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
