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