[FFmpeg-devel] [PATCH] Clip-Wrapped MXF support (attempt #4)
Tomas Härdin
tomas.hardin
Mon Jan 24 11:41:22 CET 2011
Maksym Veremeyenko skrev 2011-01-13 13:14:
> Hi,
>
> This is another attempt to submit patches for clip-wrapped MXF files
> support (generated by Panasonic's P2 camera).
>
> Patches set:
>
> *0001-add-MXFContainerUL-struct-for-containers-UL-dict.patch* - start
> use MXFContainerUL instead of MXFCodecUL struct for essence containers
> list.
>
> *0002-revert-container-wrapping-detection-from-r11567.patch* - revert
> almost all optimization of clip-wrapped detection from r11567.
>
> *0003-make-key-output-in-RP-224.10-form.patch* - on output debug use
> RP224 presentation UID for containers and codec - dot separated hex values.
>
> *0004-add-new-essence-container-uls.patch* - extends well-known (IMHO)
> uid for DV videos and PCM audios recorded by P2 camera.
>
> *0005-extend-MXFIndexTableSegment.patch* - extends reading datas for
> MXFIndexTableSegment
>
> *0006-clip-wrapped-support-added-for-single-track-file.patch* - actually
> a $subj...
>
> Please commit, comment or reject...
> From 0c0102d64c0a0d75bd2130d300e0e7609beb62ad Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko <verem at m1.tv>
> Date: Sun, 19 Sep 2010 12:10:22 +0300
> Subject: [PATCH 1/6] add MXFContainerUL struct for containers UL dict
>
> ---
> libavformat/mxfdec.c | 25 +++++++++++++++++++++----
> 1 files changed, 21 insertions(+), 4 deletions(-)
Looks OK to me.
> From 6127bb2ebba4fb920a578f4ec7c16bee397793c5 Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko <verem at m1.tv>
> Date: Sun, 19 Sep 2010 12:14:24 +0300
> Subject: [PATCH 2/6] revert container wrapping detection from r11567
>
> ---
> libavformat/mxfdec.c | 20 +++++++++++++-------
> 1 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 6bc6b77..f840e1f 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -668,13 +668,19 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMe
>
> static const MXFContainerUL mxf_essence_container_uls[] = {
> // video essence container uls
> - { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x60,0x01 }, 14, CODEC_ID_MPEG2VIDEO }, /* MPEG-ES Frame wrapped */
> - { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x41,0x01 }, 14, CODEC_ID_DVVIDEO }, /* DV 625 25mbps */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x60,0x01 }, 16, CODEC_ID_MPEG2VIDEO, Frame }, /* MPEG-ES Frame wrapped */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0xe0,0x02 }, 16, CODEC_ID_MPEG2VIDEO, Clip }, /* MPEG-ES Clip wrapped, 0xe0 MPV stream id */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x04,0x61,0x07 }, 16, CODEC_ID_MPEG2VIDEO, Clip }, /* MPEG-ES Custom wrapped, 0x61 ??? stream id */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x41,0x01 }, 16, CODEC_ID_DVVIDEO, Frame }, /* DV 625 25mbps */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x3F,0x01 }, 16, CODEC_ID_DVVIDEO, Frame }, /* DV IEC 625 25mbps */
> // sound essence container uls
> - { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x06,0x01,0x00 }, 14, CODEC_ID_PCM_S16LE }, /* BWF Frame wrapped */
> - { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x40,0x01 }, 14, CODEC_ID_MP2 }, /* MPEG-ES Frame wrapped, 0x40 ??? stream id */
> - { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x01,0x01,0x01 }, 14, CODEC_ID_PCM_S16LE }, /* D-10 Mapping 50Mbps PAL Extended Template */
> - { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, 0, CODEC_ID_NONE },
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x06,0x01,0x00 }, 16, CODEC_ID_PCM_S16LE, Frame }, /* BWF Frame wrapped */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x06,0x03,0x00 }, 16, CODEC_ID_PCM_S16LE, Frame }, /* AES Frame wrapped */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0x40,0x01 }, 16, CODEC_ID_MP2, Frame }, /* MPEG-ES Frame wrapped, 0x40 ??? stream id */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0xc0,0x01 }, 16, CODEC_ID_MP2, Frame }, /* MPEG-ES Frame wrapped, 0xc0 MPA stream id */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x04,0xc0,0x02 }, 16, CODEC_ID_MP2, Clip }, /* MPEG-ES Clip wrapped, 0xc0 MPA stream id */
> + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x01,0x01,0x01 }, 14, CODEC_ID_PCM_S16LE, Frame }, /* D-10 Mapping 50Mbps PAL Extended Template */
> + { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, 16, CODEC_ID_NONE, Frame },
> };
This causes a lot of DV samples to fail. For instance, try making an
OPAtom file by running http://samples.mplayerhq.hu/DV-raw/voxnews.dv
through mxfwrap -a (in MXFLib) and you'll get an UL where byte 14 is 0x7F.
> static int mxf_parse_structural_metadata(MXFContext *mxf)
> @@ -850,7 +856,7 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
> st->need_parsing = AVSTREAM_PARSE_FULL;
> }
> }
> - if (st->codec->codec_type != AVMEDIA_TYPE_DATA && (*essence_container_ul)[15] > 0x01) {
> + if (st->codec->codec_type != AVMEDIA_TYPE_DATA && container_ul && container_ul->wrapping == Clip) {
Yes, the above check is made prettier. However, the loss of generality
in mxf_essence_container_uls might not be worth it.
> From 460438fc89f813bbaf7703b351ab675236c95a8d Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko <verem at m1.tv>
> Date: Sun, 19 Sep 2010 12:17:00 +0300
> Subject: [PATCH 3/6] make key output in RP 224.10 form
>
> ---
> libavformat/mxf.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
> index 26832c2..5de949e 100644
> --- a/libavformat/mxf.h
> +++ b/libavformat/mxf.h
> @@ -71,7 +71,7 @@ extern const MXFPixelLayout ff_mxf_pixel_layouts[];
> int ff_mxf_decode_pixel_layout(const char pixel_layout[16], enum PixelFormat *pix_fmt);
>
> #ifdef DEBUG
> -#define PRINT_KEY(pc, s, x) dprintf(pc, "%s %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X\n", s, \
> +#define PRINT_KEY(pc, s, x) dprintf(pc, "%s %02X.%02X.%02X.%02X.%02X.%02X.%02X.%02X %02X.%02X.%02X.%02X.%02X.%02X.%02x.%02X\n", s, \
This seems completely unrelated. I'd drop this patch from this thread
and have it in a thread of its own since it's a bit distracting.
> From 3fad6363f12bc0346e66e9592112f8000b579118 Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko <verem at m1.tv>
> Date: Sun, 19 Sep 2010 12:22:55 +0300
> Subject: [PATCH 5/6] extend MXFIndexTableSegment
>
> ---
> libavformat/mxfdec.c | 40 ++++++++++++++++++++++++++++++++++------
> 1 files changed, 34 insertions(+), 6 deletions(-)
Looks OK to me.
> From 1ee036697cde16303850d1557d06d5f4caa37af6 Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko <verem at m1.tv>
> Date: Sun, 19 Sep 2010 12:30:27 +0300
> Subject: [PATCH 6/6] clip-wrapped support added for single track file
>
> ---
> libavformat/mxfdec.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 58 insertions(+), 1 deletions(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index d510e77..f77314a 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -139,6 +139,9 @@ typedef struct {
> struct AVAES *aesc;
> uint8_t *local_tags;
> int local_tags_count;
> + KLVPacket current_klv_data;
> + int current_klv_index;
> + int current_klv_bsize;
> } MXFContext;
>
> enum MXFWrappingScheme {
> @@ -317,8 +320,26 @@ static int mxf_decrypt_triplet(AVFormatContext *s, AVPacket *pkt, KLVPacket *klv
> static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
> {
> KLVPacket klv;
> + int index;
> + MXFContext* mxf = s->priv_data;
>
> while (!url_feof(s->pb)) {
> + if (mxf->current_klv_data.length) {
> + /* store */
> + klv = mxf->current_klv_data;
> + index = mxf->current_klv_index;
> +
> + /* setup length */
> + klv.length = FFMIN(mxf->current_klv_bsize, mxf->current_klv_data.length);
> +
> + /* modify size, length */
> + mxf->current_klv_data.offset += klv.length;
> + mxf->current_klv_data.length -= klv.length;
> +
> + /* read packet */
> + goto read_data;
> + };
Looks like a decent solution.
> if (klv_read_packet(&klv, s->pb) < 0)
> return -1;
> PRINT_KEY(s, "read packet", klv.key);
> @@ -332,13 +353,49 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
> return 0;
> }
> if (IS_KLV_KEY(klv.key, mxf_essence_element_key)) {
> - int index = mxf_get_stream_index(s, &klv);
> + index = mxf_get_stream_index(s, &klv);
> if (index < 0) {
> av_log(s, AV_LOG_ERROR, "error getting stream index %d\n", AV_RB32(klv.key+12));
> goto skip;
> }
> if (s->streams[index]->discard == AVDISCARD_ALL)
> goto skip;
> +
> + /* check for clip wrapped and single track */
> + if (AVSTREAM_PARSE_FULL == s->streams[index]->need_parsing &&
> + 1 == s->nb_streams) {
This test seems dubious to me. Consider keeping a MXFContainerUL pointer
in MXFTrack instead, and checking the wrapping type.
The above code only seems to be meant to work with OPAtom files, but the
name of the thread would hint that OP1a should work as well.
> + int k;
> +
> + /* store current klv information */
> + mxf->current_klv_data = klv;
> + mxf->current_klv_index = index;
> + mxf->current_klv_bsize = 0;
> +
> + /* find block size */
> + for (k = 0; k < mxf->metadata_sets_count; k++) {
> + MXFMetadataSet *metadata = mxf->metadata_sets[k];
> + if (IndexTableSegment == metadata->type) {
> + mxf->current_klv_bsize = ((MXFIndexTableSegment *)metadata)->edit_unit_byte_count;
> + break;
> + }
> + }
Tie the index to the MXFTrack instead so multiple streams can work.
> + /* check small EditUnitByteCount for audio */
> + if (!mxf->current_klv_bsize)
> + mxf->current_klv_bsize = 1024;
> + else if (mxf->current_klv_bsize < 32)
> + mxf->current_klv_bsize *= 1024;
This seems extremely hackish. Does the specs really allow
EditUnitByteCount to be anything but samplerate/editrate? It would seem
better to error out instead.
> + av_log(s, AV_LOG_DEBUG, "Clip-wrapped reading. mxf->current_klv_bsize=%d\n",
> + mxf->current_klv_bsize);
> +
> + return mxf_read_packet(s, pkt);
> + }
> +read_data:
> + PRINT_KEY(s, "read packet data", klv.key);
> + av_log(s, AV_LOG_DEBUG, "data size %lld offset %#llx\n", klv.length, klv.offset);
dprintf()?
/Tomas
More information about the ffmpeg-devel
mailing list