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

[Stephen Backway]

On Mon, 2009-08-24 at 20:59 +0200, Michael Niedermayer wrote:
> 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
> 
> ?
The way it is now makes it easier for when the RLE data is over multiple
packets.  I would prefer to leave it this way, as it will make adding
support for this an almost trivial case (I just need to find some
subtitles that do this).
> 
> [...]
> > +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
> 
changed to rle_buf_size and rle_data_len respectively. As we are using
av_fast_malloc this are not always the same value.

> const uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;
> > like in vc1dsp.c, simple_idct.c etc
> 
> i prefer const uint8_t
Fixed. 

> either way iam fine with your patch once all the little things that
> have been raised are dealt with
> 
I think I have covered all comments.

Stephen.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bluray_subtitles_ffmpeg_19697_v12.diff
Type: text/x-patch
Size: 17701 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090825/6ac99ac0/attachment.bin>