[FFmpeg-devel] [PATCH] Bluray Subtitle Support, v2

[Aurelien Jacobs]

On Fri, Jul 24, 2009 at 05:57:24PM +1000, stev391 at exemail.com.au wrote:
> On Thu, 2009-07-23 at 23:33 +0200, Diego Biurrun wrote:
> > 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.
> > 
> 
> Updated patch including Diego's, Aurelien's & Baptiste's comments.
> Thanks for the excellent and quick feedback.
> 
> @Baptiste,
> I originally had the parser due to trying to fix the delay in display of
> the subtitle.  However as this is not utilised at the moment I have
> removed it.  I have yet to work out how to make the subtitles appear at
> the correct time. I think at the moment they appear just before they
> would normally be blanked, however late subtitles are better then none!
> 
> @Diego & @Aurelien,
> Thankyou for the comments, the code now reads a lot easier and is a
> smaller file!
> 
> [...]
> 
> +static void pgssub_parse_window_segment(AVCodecContext *avctx,
> +                                        const uint8_t *buf, int buf_size)
> +{
> +    PGSSubContext *ctx = (PGSSubContext*) avctx->priv_data;
> +    int x, y, w, h;
> +
> +    /* Skip 2 bytes of unknown, */
> +    buf += 2;
> +
> +    x = bytestream_get_be16(&buf);
> +    y = bytestream_get_be16(&buf);
> +    w = bytestream_get_be16(&buf);
> +    h = bytestream_get_be16(&buf);
> +
> +    dprintf(avctx, "Window dimensions x=%d, y=%d, w=%d, h=%d\n", x, y, w, h);
> +
> +    ctx->window->x = x;
> +    ctx->window->y = y;
> +    ctx->window->w = w;
> +    ctx->window->h = h;

Here (and probably at some other places) you could merge the assignment.
Something like this:

    x = ctx->window->x = bytestream_get_be16(&buf);
    y = ctx->window->y = bytestream_get_be16(&buf);
    w = ctx->window->w = bytestream_get_be16(&buf);
    h = ctx->window->h = bytestream_get_be16(&buf);

And in this specific case, I think the x, y, w and h temporary variables
are useless...

Aurel