[FFmpeg-devel] [PATCH] libavcodec/pgx: Added pgx decoder

[Gautam Ramakrishnan]

On Fri, Jun 26, 2020 at 3:50 AM Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
>
> Am Do., 25. Juni 2020 um 09:23 Uhr schrieb Gautam Ramakrishnan
> <gautamramk at gmail.com>:
> >
> > On Thu, Jun 25, 2020 at 12:50 AM Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
>
> > > > +    for (i = 0; i < 6; i++) {
> > > > +        if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) {
> > > > +            return AVERROR_INVALIDDATA;
> > > > +        }
> > >
> > > Use memcmp() or consider to drop this check:
> > > If a user forces this decoder, it should not be necessary to depend on this.
> > >
> > I did not understand this
>
> My personal believe is - although some decoders are doing this - that
> this check is not useful.
> If you keep the check, it should be done with memcmp().
>
> [...]
>
> > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > >
> > > I believe other developers will request that you split the demuxer
> > > and the decoder patch.
> > >
> > The only issue is the demuxer refers to the decoder. How do I do this?
>
> If you decide to split the patch, the first patch adds the decoder,
> the second the demuxer.
> Please increase MINOR versions for both libraries in the appropriate
> patches.
>
> [...]
>
> > > > +static int pgx_probe(const AVProbeData *p)
> > > > +{
> > > > +    const uint8_t *b = p->buf;
> > > > +    int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000;
> > >
> > > if (AV_RB24(b) != ...)
> > > return 0;
> > >
> > > > +    ret = ret && av_match_ext(p->filename, "pgx");
> > > > +    if (ret)
> > > > +        return AVPROBE_SCORE_EXTENSION + 1;
> > >
> > > You should instead check if the file internally looks like pgx,
> > > checking the extension is a final possibility for things that
> > > are impossible to detect (which I think is not the case here).
> > >
> > The previous check does that. It checks if the first 3 bytes are PG and space.
>
> Please check more than three bytes - use memcmp() - and please
> remove the usage of av_match_ext(), it should only be used in rare
> cases, this is not one of them.

I found the instructions to add a new codec. It says add an entry to
libavcodec/codec_desc.c. I understand this CoDec would have a lossless
property. What other properties should this codec have?



-- 
-------------
Gautam |