Just use
HDMV Interactive Graphics Stream decoder
as log message subject.
On Fri, Jun 08, 2012 at 01:33:22PM +0200, David Girault wrote:
> This new decoder will allow to decode menu stream found
streamS
> in m2ts files in Bluray or AVCHD HDMV disks.
>
> The new decoder was developped using AVMEDIA_TYPE_OVERLAY,
> inspired from AVMEDIA_TYPE_SUBTITLE, since both may send
> multiple decoded pictures to add to video currently
> displayed.
This remark is informative wrt the history of this patch, but there is
no point in keeping it in the log message.
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -201,6 +201,7 @@ OBJS-$(CONFIG_IAC_DECODER) += imc.o
> OBJS-$(CONFIG_IDCIN_DECODER) += idcinvideo.o
> OBJS-$(CONFIG_IFF_BYTERUN1_DECODER) += iff.o
> OBJS-$(CONFIG_IFF_ILBM_DECODER) += iff.o
> +OBJS-$(CONFIG_IGSMENU_DECODER) += igsmnudec.o
Let's keep the 'e' from "menu" in the filename.
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -375,6 +375,9 @@ void avcodec_register_all(void)
>
> + /* specific data */
> + REGISTER_DECODER (IGSMENU, igsmenu);
again: Why "specific"?
> --- /dev/null
> +++ b/libavcodec/igsmnudec.c
> @@ -0,0 +1,787 @@
> +
> +#define YUVA( y, u, v, a ) ( ((a) << 24) | ((y) << 16) | ((u) << 8) | (v) )
no spaces inside ()
> +/**
> + * Decode the RLE data.
> + *
> + * The bitmap is stored as an Run Length Encoded image.
> + *
> + * @param avctx contains the current codec context
> + * @param ove pointer to the processed overlay data
> + * @param buf pointer to the RLE data to process
> + * @param buf_size size of the RLE data to process
> + * @return a negative value on error, otherwise return 0
> + */
> +static int decode_rle(AVCodecContext *avctx, AVOverlay *ove, int i,
> + const uint8_t *buf, unsigned int buf_size)
"i" is a bad parameter name and its documentation is missing.
> + if (run > 0 && pixel_count + run <= ove->rects[i]->w *
> ove->rects[i]->h) {
> + memset(ove->rects[i]->pict.data[0] + pixel_count, color, run);
> + pixel_count += run;
> + }
> + else
} else
> + av_dlog(avctx, " Pixel Count = %d, Area = %d\n", pixel_count,
> ove->rects[i]->w * ove->rects[i]->h);
Break this line.
> +static int parse_picture_segment(AVCodecContext *avctx,
> + const uint8_t *buf, int buf_size)
> +{
> + /* Decode rle bitmap length, stored size includes width/height data */
> + rle_bitmap_len = bytestream_get_be24(&buf) - 2*2;
spaces around *
> + pic = av_realloc(ctx->pictures, ctx->pictures_count *
> sizeof(IGSPicture));
> + if (pic == NULL) {
if (!pic) {
> + pic->rle_data_len = buf_size;
> + pic->rle_remaining_len = rle_bitmap_len - buf_size;
Align the =
> + av_dlog(avctx, " Color %d := (%d,%d,%d,%d)\n", color_id, y, cr, cb,
> alpha);
Break this line.
> +/**
> + * Parse the button segment packet.
> + * The button segment contains pages and details on the interactive graphics.
> + *
> + * @param avctx contains the current codec context
> + * @param buf pointer to the packet to process
> + * @param buf_size size of packet to process
> + * @return a negative value on error, otherwise return 0
> + * @todo TODO: Implement cropping
Adding "TODO:" here is redundant.
> + bog_cnt = bytestream_get_byte(&buf);
> + av_dlog(avctx, "Page %d @ %X has %d BOGs, use palette %d\n",
> + i, ((int)(buf-start))-21, bog_cnt, page->palette);
spaces around -
> +/**
> + * Parse 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
> + * @return return 1 if something to display, else 0
> + * @todo Fix start time, relies on correct PTS and IN_time value.
> + * Fix end time, normally cleared by a second display
> + * segment or user timeout for popup menu.
> + */
Please align the parameter name descriptions; this also applies to the
other Doxygen blocks.
> + if (buf_size >= 2) {
> + if (buf[1] < ctx->pages_count)
> + ctx->page = buf[1];
> + }
> + else {
} else {
> + /* just cycling page without parameters */
> + if (++ctx->page >= ctx->pages_count)
> + ctx->page=0;
spaces around =
> + }
> + page = &ctx->pages[ctx->page];
> + if (buf_size >= 4)
> + page->sel_button = *(const uint16_t*)(buf+2);
spaces around +
Please avoid pointer type punning, there must be a proper way to
achieve the same thing.
> + if (button) {
> + uint16_t next = button->id;
> + switch(*buf) {
switch (
> + ove->num_rects = page->bogs_count;
> +
> + av_log(avctx, AV_LOG_INFO, "Displaying menu page %d, %d bog, button %d
> %s.\n",
> + ctx->page, ove->num_rects, page->sel_button,
> (activated?"activated":"selected"));
Break these lines.
> + if (!picture) {
> + // bitmap not found for that button!!!
> + rect->w = 0;
> + rect->h = 0;
> + }
> + else {
> + // process bitmap
} else {
> + rect->w = picture->w;
> + rect->h = picture->h;
> + rect->pict.linesize[0] = picture->w;
Align the '='.
> + if (picture->rle) {
> + if (picture->rle_remaining_len)
> + av_log(avctx, AV_LOG_ERROR,
> + "RLE data length %u is %u bytes shorter than
> expected\n",
> + picture->rle_data_len,
> picture->rle_remaining_len);
> + if(decode_rle(avctx, ove, i, picture->rle,
> picture->rle_data_len) < 0) {
if (
> + if ((page->sel_button == button->id) && activated) {
> + // TODO: button HDMV program must be included when activated
> + if (button->cmds_count > 0) {
> + ove->extra_sz = 3 * sizeof(uint32_t) * button->cmds_count;
> + ove->extra = av_mallocz(ove->extra_sz);
Align the '='.
> +static int decode(AVCodecContext *avctx, void *data, int *data_size,
> + AVPacket *avpkt)
> +{
> + if (ret<0) {
spaces around <
> + int i;
> + av_log(avctx, AV_LOG_DEBUG, "Error returned for segment length
> %d with type %x\n", segment_length, segment_type);
Break this line.
> +AVCodec ff_igsmenu_decoder = {
> + .init = init_decoder,
> + .close = close_decoder,
> + .decode = decode,
These function names are much too generic.
igsmenu_init
igsmenu_close
igsmenu_decode
should be suitable replacement names.
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1368,6 +1368,39 @@ void avsubtitle_free(AVSubtitle *sub)
> +void avoverlay_free(AVOverlay *overlay)
> +{
> +
> + av_freep(&overlay->rects);
> + if (overlay->extra)
> + av_freep(&overlay->extra);
stray tabs
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
