Re: [FFmpeg-devel] [PATCH 5/6] avcodec/mpeg4videodec: Remove always-true checks

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 5/6] avcodec/mpeg4videodec: Remove always-true checks
Date: Fri, 14 Oct 2022 00:22:05 +0200
Message-ID: <20221013222205.GC1970738@pb2> (raw)
In-Reply-To: <AS8P250MB07447CC6D250AE48BABA638D8F259@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM>

[-- Attachment #1.1: Type: text/plain, Size: 1919 bytes --]

On Thu, Oct 13, 2022 at 11:23:33PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Hi
> > 
> > On Wed, Oct 12, 2022 at 08:06:22PM +0200, Andreas Rheinhardt wrote:
> >> codec_id is always AV_CODEC_ID_MPEG4 in this file.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> >> ---
> >>  libavcodec/mpeg4videodec.c | 6 ++----
> >>  1 file changed, 2 insertions(+), 4 deletions(-)
> > [...]
> >> @@ -3084,7 +3083,6 @@ int ff_mpeg4_workaround_bugs(AVCodecContext *avctx)
> >>                 ctx->divx_version, ctx->divx_build, s->divx_packed ? "p" : "");
> >>  
> >>      if (CONFIG_MPEG4_DECODER && ctx->xvid_build >= 0 &&
> >> -        s->codec_id == AV_CODEC_ID_MPEG4 &&
> > 
> > should the CONFIG_MPEG4_DECODER check be removed too ?
> > 
> 
> This file is also compiled if the MPEG-4 parser or the H.263 decoder is
> enabled (hence if any of the H.263 decoder family is enabled); the
> former is probably done because of ff_mpeg4_pred_ac() which is used by
> msmpeg4dec.c (this is the only exception to what I wrote in the commit
> message; will amend it). The latter could be solved by moving the code
> common to mpeg4-decoder and parser out into a file of its own, say
> mpeg4_parse.c. I pondered doing that; would you have any objections to
> it? It would allow to cut down on the crazy list of requirements of the
> mpeg4 parser.

really, you are doing this factorization, you decide which way it makes
most sense.
Id just say something if i see something that "feels" wrong
here it was just me seeing te compile time check and you saying its always
AV_CODEC_ID_MPEG4.
Maybe you could word the commit messsage more verbosely 

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".