[FFmpeg-devel] [PATCH 3/5] avcodec/mpeg4videode: Eliminate out of loop VOP startcode reading for studio profile

Michael Niedermayer michael at niedermayer.cc
Thu May 3 02:40:07 EEST 2018


On Sun, Apr 29, 2018 at 11:24:19PM +0000, Kieran Kunhya wrote:
> On Sun, 29 Apr 2018 at 20:20 Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/mpeg4videodec.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> > index 9ee2f37c69..9318d7bd4e 100644
> > --- a/libavcodec/mpeg4videodec.c
> > +++ b/libavcodec/mpeg4videodec.c
> > @@ -2931,9 +2931,6 @@ static int decode_studio_vop_header(Mpeg4DecContext
> > *ctx, GetBitContext *gb)
> >      if (get_bits_left(gb) <= 32)
> >          return 0;
> >
> > -    if (get_bits_long(gb, 32) != VOP_STARTCODE)
> > -        return AVERROR_INVALIDDATA;
> > -
> >      s->decode_mb = mpeg4_decode_studio_mb;
> >
> >      decode_smpte_tc(ctx, gb);
> > @@ -3183,7 +3180,6 @@ int ff_mpeg4_decode_picture_header(Mpeg4DecContext
> > *ctx, GetBitContext *gb)
> >              if (s->studio_profile) {
> >                  if ((ret = decode_studio_vol_header(ctx, gb)) < 0)
> >                      return ret;
> > -                break;
> >              } else {
> >                  if ((ret = decode_vol_header(ctx, gb)) < 0)
> >                      return ret;
> > --
> > 2.17.0
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> I appreciate you are going to ram this through as "maintainer" but many of
> these changes do not reflect the intentions of the specification and build
> on bad design decisions in mpeg4videodec from 10-15 years ago.

Id like to move forward, the rest of this patchset as well as the remaining
patch of the other set depend on this.

If you have technical arguments iam interrested to hear them. I do not share
your view though.
The specification, or rather the authors of the specification did not had the
intend that every line would be 1:1 copied into an implementation. I think this
is not even done in the standard comitees reference implementations in general.

About the past, it appears to me that there is a deeply rooted aversion by
some people toward some code. This just doesnt belong here.

The parsing of a format or codec by a main loop and a "switch" on the "chunk"
type is very common. This style is used in modern code as well. And probably
was in many places used as a response towards to practical problems with 
more "spec like" rigid parsing. At least for avi and wav i remember that
applications using strict parsing had issues playing some files ...

About this here more specifically, the old code can handle basically every
mpeg4 file. The new studio_profile code fails to play every of the reference
streams, it does only play the 3 non reference streams we have

So if there are no objections i will in a day or 2 apply this and the rest
of the patchset(s)

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180503/9a45c775/attachment.sig>


More information about the ffmpeg-devel mailing list