[FFmpeg-devel] [PATCH v5] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720

[Moritz Barsnick]

Some minor nitpicking remarks from me:

> Subject: [PATCH] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720.

Basically, the second sentence belongs into the body of the commit
message, after a single empty line separator. (It may not matter here.)

>  Changelog                |  1 +
>  libavformat/Makefile     |  1 +
>  libavformat/allformats.c |  1 +
>  libavformat/flvdec.c     | 85 ++++++++++++++++++++++++++++++++++++++++

You could also add a documentation entry for xv, referring to the flv
doc. That would be in doc/demuxers.texi. (Can be added later, IMO.)

> +    if (d[0] == 'X' &&
> +        d[1] == 'L' &&
> +        d[2] == 'V' &&
> +        d[3] == 'F') {

Question to the other core developers: Is there any preference for this
style of code over:
       if (AV_RL32(d) == MKTAG('X', 'L', 'V', 'F')
which some other probes use?
(flv rightly doesn't use the latter style, because d[3] is variable.)

> +        return AVPROBE_SCORE_EXTENSION + 1;

I believe Carl Eugen asked you to change this to:
AVPROBE_SCORE_EXTENSION / 2 + 1

> +    if (!strcmp(s->iformat->name , "xv")) {
> +        for (i=0; i < FFMIN(2,fileposlen); i++) {
                i = 0 (whitespace around all operators)
> +};
> \ No newline at end of file

You should also consider adding a newline at the end of the last line.

If this works (I haven't tested): Nice improvement versus your first
patch!

Cheers,
Moritz