[FFmpeg-devel] mpeg12dec fix up DVD caption handling

Moritz Barsnick barsnick at gmx.net
Tue Sep 13 22:42:33 EEST 2016


On Mon, Sep 12, 2016 at 18:19:43 -0700, Jonathan Campbell wrote:
> Subject: [PATCH 2/7] read caption words field-wise, count properly and limit
>  to cc_count. transfer each CC word taking into consideration immediate CC
>  field bit or for DVDs that don't use it, keep track according to first field
>  specified at start of DVD caption packet.

We're apparently missing patches 3 to 6 of 7?

Anyways, commit messages should consist of a first line as summary (not
too long) with a prefix such as "lavc/mpeg12dec: ", then an empty line
separating the following optional, but possibly verbose commit
explanation or continuation. Don't try to squeeze mulitple sentences
into the commit line. (Check other commits for reference.)

> +            if ((p[i] & 0xfe) != 0xfe) {
> +                av_log(avctx, AV_LOG_DEBUG, "cc_count is too large (%u > %u) or junk data in DVD caption packet",(unsigned int)caption_block_count,(unsigned int)cc_count);

I don't see the need to cast ints to unsigned ints for printing. Either
just print with %d, or use unsigned ints in the first place if you
never (ever) need negative values.

> +                    /* if the source actually uses the caption_odd_field bit, then use that to determine the field.
> +                     * else, toggle between fields to keep track for DVDs where p[0] == 0xFF at all times. */

Very nitpicky: If you use full sentences ending in a period, then also
begin them with capitals, like writing proper text. Else my eyes hurt.
(Or: Else it's hard finding the beginning.)
;-)

Moritz


More information about the ffmpeg-devel mailing list