[FFmpeg-devel] [PATCH] attachments support in matroska demuxer

[Michael Niedermayer]

Hi

On Sat, Dec 29, 2007 at 10:38:24PM +0300, Evgeniy Stepanov wrote:
> Hi,
> 
> this patch adds attachments support to mkv demuxer. They are collected in 
> AVFormatContext::attachments. This is needed to correctly display SSA/ASS 
> subtitles that often depend on fonts emdedded in matroska container.

just for the case that we end up deciding to add AVAttachments
here are a few comments about things which would need to be fixed


[...]

> Index: libavformat/avformat.h
> ===================================================================
> --- libavformat/avformat.h	(revision 11346)
> +++ libavformat/avformat.h	(working copy)
> @@ -371,6 +371,15 @@
>  
>  #define MAX_STREAMS 20
>  
> +typedef struct AVAttachment
> +{
> +    char* name;
> +    char* mime;
> +    uint64_t uid;
> +    void* data;
> +    unsigned int data_size;
> +} AVAttachment;

char *name
char *type
union value{
    int64_t integer;
    AVRational rational;
    struct{
        uint8_t *data;
        unsigned int data_size;
    }
}

what good does uid do? (IMO it should be removed)
Also doxygen comments are needed for all fields explaining what they are.


> +
>  /**
>   * format I/O context.
>   * New fields can be added to the end with minor version bumps.
> @@ -476,6 +485,9 @@
>       * demuxing: set by user
>       */
>      enum CodecID subtitle_codec_id;
> +
> +    unsigned int nb_attachments;
> +    struct AVAttachment* attachments;

no this breaks ABI extensibility, that is we couldnt add a field to 
AVAttachment without breaking ABI

also code passing AVAttachment to (the correct) decoders is missing

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20080102/853da3ea/attachment.pgp>