[FFmpeg-devel] [PATCH] Bluray Subtitle Support, v7 (Updated patch to v11)

[Michael Niedermayer]

On Mon, Aug 24, 2009 at 08:08:18PM +1000, Stephen Backway wrote:
[...]
> > [...]
> > > +/**
> > > + * Parses the display segment packet.
> > > + *
> > > + * The display segment controls the updating of the display.
> > > + *
> > > + * @param avctx contains the current codec context
> > > + * @param data pointer to the data pertaining the subtitle to display
> > > + * @param buf pointer to the packet to process
> > > + * @param buf_size size of packet to process
> > > + * @todo TODO: Fix start time, relies on correct PTS, currently too late
> > > + *
> > > + * @todo TODO: Fix end time, normally cleared by a second display
> > > + * @todo       segment, which is currently ignored as it clears
> > > + * @todo       the subtitle too early.
> > > + */
> > > +static int display_end_segment(AVCodecContext *avctx, void *data,
> > > +                               const uint8_t *buf, int buf_size)
> > > +{
> > > +    AVSubtitle    *sub = data;
> > > +    PGSSubContext *ctx = avctx->priv_data;
> > > +
> > > +    /*
> > > +     *      The end display time is a timeout value and is only reached
> > > +     *      if the next subtitle is later then timeout or subtitle has
> > > +     *      not been cleared by a subsequent empty display command.
> > > +     */
> > > +
> > > +    sub->start_display_time = 0;
> > > +    sub->end_display_time   = 20000;
> > > +    sub->format             = 0;
> > > +
> > > +    if (!sub->rects) {
> > > +        sub->rects     = av_mallocz(sizeof(*sub->rects));
> > > +        sub->rects[0]  = av_mallocz(sizeof(*sub->rects[0]));
> > > +        sub->num_rects = 1;
> > > +    }
> > > +
> > > +    sub->rects[0]->x    = ctx->presentation.x;
> > > +    sub->rects[0]->y    = ctx->presentation.y;
> > > +    sub->rects[0]->w    = ctx->picture.w;
> > > +    sub->rects[0]->h    = ctx->picture.h;
> > > +    sub->rects[0]->type = SUBTITLE_BITMAP;
> > > +
> > > +    /* Allocate memory for bitmap */
> > > +    sub->rects[0]->pict.data[0]     = av_malloc(ctx->picture.w * ctx->picture.h);
> > > +    sub->rects[0]->pict.linesize[0] = ctx->picture.w;
> > > +
> > > +    if (ctx->picture.bitmap)
> > > +        memcpy(sub->rects[0]->pict.data[0], ctx->picture.bitmap, ctx->picture.w * ctx->picture.h);
> > 
> > cant the image be drawn into sub->rects[0]->pict.data[0] instad of bitmap to
> > avoid that memcpy ?
> As the packet containing the RLE image data and the packet that issues
> the display command are in seperate instances I need at least one memcpy
> to store either the RLE data or the decoded bitmap in the context.

hmm, where is the problem with
packet 1
    * alloc picture
    * decode rle into picture
packet 2
    * return picture

?

[...]
> +typedef struct PGSSubPicture {
> +    int          w;
> +    int          h;
> +    uint8_t      *rle;

> +    unsigned int rle_size, rle_len;

these 2 are confusing names, they are so similar, maybe you could
find clearer ones or at least document them

either way iam fine with your patch once all the little things that
have been raised are dealt with

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090824/4d272088/attachment.pgp>