[FFmpeg-devel] [PATCH] Bluray Subtitle Support

[Diego Biurrun]

On Thu, Jul 23, 2009 at 07:16:21PM +1000, stev391 at exemail.com.au wrote:
> 
> Please find attached a patch that adds Bluray subtitle support.

This patch is missing a documentation and changelog update.

> --- libavcodec/pgssubdec.c	(revision 0)
> +++ libavcodec/pgssubdec.c	(revision 0)
> @@ -0,0 +1,579 @@
> +/*
> + * PGS subtitle decoding for ffmpeg

You can leave out "for ffmpeg".

> +/**
> + * @file libavcodec/pgssubdec.c
> + * PGS Subtitle Decoder

subtitle decoder

> +static void pgssub_parse_picture_segment(AVCodecContext *avctx,
> +                                        const uint8_t *buf, int buf_size)

indentation

> +    uint8_t block;
> +    uint8_t flags;
> +    uint8_t colour;
> +    uint8_t *rle_bitmap_end;
> +    uint8_t sequence_desc;

These five lines could be one or two.

> +    pixel_count = 0;
> +    line_count = 0;

align

> +        }
> +        else {

Merge these two lines, same below.

> +            if (pixel_count % width == width - 1) {
> +                av_log(avctx, AV_LOG_ERROR, "Decoded %d pixels, when line should be %d pixels\n",
> +                       pixel_count % width, width);
> +            }

pointless {}

> +#ifdef DEBUG
> +    av_log(avctx, AV_LOG_INFO, "Pixel Count = %d, Area = %d\n", pixel_count, width * height);
> +#endif

dprintf, same below

> +static void pgssub_parse_palette_segment(AVCodecContext *avctx,
> +                                        const uint8_t *buf, int buf_size)

indentation

> +    for (i=0; i < buf_size; i++) {
> +        av_log(avctx, AV_LOG_INFO, "%02x ", buf[i]);
> +    }

pointless {} and this is the only place where you do not have spaces
around '='.

> +        /* Skip 7 bytes (+1 for previous byte) of unknown:
> +               pallete_update_flag (0x80),
> +               pallete_id_to_use,

paLeTTe

> +#if 0

This is bad.

> +    for (i=0; i < buf_size; i++) {
> +        av_log(avctx, AV_LOG_INFO, "%02x ", buf[i]);
> +    }

pointless {}

Actually you always skip spaces around '=' in for-constructs, but
nowhere else..

> +static int pgssub_display_end_segment(AVCodecContext *avctx,
> +                                        void *data,
> +                                        const uint8_t *buf, int buf_size)

indentation

> --- libavcodec/pgssub_parser.c	(revision 0)
> +++ libavcodec/pgssub_parser.c	(revision 0)
> @@ -0,0 +1,128 @@
> +/*
> + * PGS subtitle parser for FFmpeg

ditto

> +/**
> + * @file libavcodec/pgssubdec.c
> + * PGS Subtitle Decoder

mindless copy and paste

> +    for (i=0; i < buf_size; i++)
> +    {

for (...) {

Diego