[FFmpeg-devel] [PATCH v2] Added XV Support

Carl Eugen Hoyos ceffmpeg at gmail.com
Mon Apr 8 01:12:29 EEST 2019


2019-04-07 8:53 GMT+02:00, Shivam Goyal <shivgo at iitk.ac.in>:
> This time i modified the initial buffer at the time of reading
> header instead of changing the IO layer.
>
> Suggest any changes required.

Please mention ticket #3720 in the commit message.

> +static int xv_probe(const AVProbeData *p)
> +{
> +    const uint8_t *d = p->buf;
> +
> +    if (d[0] == 'X' &&
> +        d[1] == 'L' &&
> +        d[2] == 'V' &&
> +        d[3] == 'F') {
> +        return AVPROBE_SCORE_MAX;

Should be MAX / 2 + 1 for 32 bit match.

[...]

> +    // For XV file the flv data starts from 0x200000 byte
> +    if(!strcmp(s->iformat->name , "xv")){
> +        for (i=0; i < FFMIN(2,fileposlen); i++){
> +            filepositions[i] += 0x200000;

The comment does not look super useful to me.

> +static int xv_read_header(AVFormatContext *s)

This function contains trailing whitespace that cannot
be committed to the FFmpeg repository, please remove
it.

> +    offset = ((((avio_r8(ic) + rot)&0xff)<<24) |
> +                (((avio_r8(ic) + rot)&0xff)<<16) |
> +                (((avio_r8(ic) + rot)&0xff)<<8) |
> +                (((avio_r8(ic) + rot)&0xff)))+ 0x200000;

Too many parentheses.

> +    // Will modify the current buffer, as only

I don't know if this is something a demuxer is allowed to do,
hopefully another developer will comment.

> +    // the bytes from 0x200000 to 0x200400 are needed to decode
> +    int size = ic->buf_end - ic->buf_ptr;

> +    int64_t pos = ic->pos - size;
> +    for(int i=0;i<0x400; i++){

Maybe I miss something but it looks as if only one of i and
pos should be needed, no? (And size could be merged as
well if that does not make reading the function too difficult.)
In any case, formatting should be:
for (int x = 0; x < 0x123; i++) {

> +        if(pos >= 0x200400) break;

> +        (*(ic->buf_ptr + i)) = (((*(ic->buf_ptr + i)) + rot) & 0xff );

Why is this not "ic->buf_ptr[i] = ic->buf_ptr[..."?
And assuming this is a char*, I would suspect the "& 0xff" is
not necessary.

+        pos+=1;

pos++

Carl Eugen


More information about the ffmpeg-devel mailing list