Set AV_DISPOSITION_ATTACHED_PIC for MKV Cover Art

[malaterre]

minidlna currently uses the following piece of code to find Cover Art:

​https://sourceforge.net/p/minidlna/git/ci/master/tree/libav.h#l171
[...]

if (s->disposition & AV_DISPOSITION_ATTACHED_PIC &&

s->codec->codec_id == AV_CODEC_ID_MJPEG)

[...]

However the flag AV_DISPOSITION_ATTACHED_PIC is never set on MKV. Steps:

Go here for MKV details on Cover Art mecanism: ​http://matroska.org/technical/cover_art/index.html

Download sample:

$ wget ​https://sourceforge.net/projects/matroska/files/test_files/cover_art.mkv
$ ffprobe -show_streams cover_art.mkv
[...]
[STREAM]
index=5
codec_name=mjpeg
codec_long_name=MJPEG (Motion JPEG)
profile=unknown
codec_type=attachment
codec_time_base=0/1
codec_tag_string=[0][0][0][0]
codec_tag=0x0000
id=N/A
r_frame_rate=0/0
avg_frame_rate=0/0
time_base=1/90000
start_pts=0
start_time=0.000000
duration_ts=14040000
duration=156.000000
bit_rate=N/A
max_bit_rate=N/A
bits_per_raw_sample=N/A
nb_frames=N/A
nb_read_frames=N/A
nb_read_packets=N/A
DISPOSITION:default=0
DISPOSITION:dub=0
DISPOSITION:original=0
DISPOSITION:comment=0
DISPOSITION:lyrics=0
DISPOSITION:karaoke=0
DISPOSITION:forced=0
DISPOSITION:hearing_impaired=0
DISPOSITION:visual_impaired=0
DISPOSITION:clean_effects=0
DISPOSITION:attached_pic=0
TAG:filename=small_cover_land.jpg
TAG:mimetype=image/jpeg
STREAM

[Hendrik]

Matroska uses codec_type == AVMEDIA_TYPE_ATTACHMENT, with mimetype and filename in metadata, since it can carry any generic attachments, and not just images. The code should probably be updated to use that in addition to the current check.

[Carl Eugen Hoyos]

Replying to malaterre:

minidlna currently uses the following piece of code to find Cover Art:

​https://sourceforge.net/p/minidlna/git/ci/master/tree/libav.h#l171
[...]

if (s->disposition & AV_DISPOSITION_ATTACHED_PIC &&

s->codec->codec_id == AV_CODEC_ID_MJPEG)

Shouldn't this be:

    if ((s->disposition & AV_DISPOSITION_ATTACHED_PIC || s->codec->codec_type == AVMEDIA_TYPE_ATTACHMENT) &&
        s->codec->codec_id == AV_CODEC_ID_MJPEG)

[malaterre]

I believe the comment #4423#comment:1 is correct. Having proper support for AV_DISPOSITION_ATTACHED_PIC in MKV will allow transcoding of Cover Art to and from MP4.

[Hendrik]

My comment means you should update minidlna code to accept codec_type == AVMEDIA_TYPE_ATTACHMENT. Matroska attachments are very generic, and the attached pic disposition requires quite specific handling, so trying to make Matroska support that would end up in quite a lot of special-case hackery.

[gjdfgh]

Cover art in Matroska is never a video stream. In libavformat they are - so it requires new code to export them correctly, which to my knowledge, libavformat doesn't do yet.

What the hell does this have to do with the codec? Don't compare codec IDs. It has nothing to do with it in this case. Just stop.

[gjdfgh]

PS: I might send a patch to export these correctly some time.

[gjdfgh]

Patch sent: [PATCH] matroskadec: export cover art correctly

Seems like the codec IDs were just looked up from the mime type (instead of frankendetected by libavformat/utils.c), so that was sufficiently correct, except there are more codecs than just jpg.

[gjdfgh]

The patch was merged. mkv coverart should now behave exactly as with other file formats in libavformat.