Baptiste Coudurier <[email protected]> added the comment:
Hi, On 01/26/2010 10:27 AM, Philip de Nier wrote: > > Philip de Nier<[email protected]> added the comment: > > Hi, > > Please find attached a patch that adds support for the 8-bit BBC Ingex Archive > MXF file format. It also adds support for the 10-bit format which is used with > DigiBeta tapes. > > A 10-bit sample file has been uploaded to here: > ftp://upload.ffmpeg.org/MPlayer/incoming/bbc-ingex-archive-10bit.mxf > ftp://upload.ffmpeg.org/MPlayer/incoming/bbc-ingex-archive-10bit.mxf.txt > > > To answer the questions raised in the previous messages: > * Re. track with no sequence: section C.1 of SMPTE 377M-2004 describes how > timecode tracks within the material package could reference a timecode > component > without a sequence inbetween > * Re. the essence container label: the essence container label for 625 line > uncompressed pictures is listed in SMPTE Rp224. Bits 0 and 1 of the last byte > needs to be masked to get the interpretation provided in the specification, > i.e. > 0x05& 0x03 gives 0x01 which indicates frame wrapping. > > > Patch notes: > * added CODEC_ID_V210 to list of intra formats in utils.c to support use of > "-vcodec copy" > * distinguishing between audio sampling rate and container sample rate which > are > different things > * reading the container sample rate from the file descriptor > * reading component depth (8/10 bit) from the file descriptor > * added uncompressed picture essence container label > * removed incorrect use of audio-only bits_per_sample in video codec section > * setting sample_aspect_ratio for raw video > * added uncompressed 8-bit and 10-bit picture essence coding labels > * selecting V210 codec and YUV422P16LE pixel format when picture essence > coding > label signals V210 packing or component depth is 10-bit > * selecting raw video and UYVY422 pixel format when component depth is 8-bit > (not 10-bit) > * handling non-byte audio bits_per_sample, e.g. 20-bit > * masking bits 0 and 1 of essence container label to determine wrapping type > > > Thanks. > Philip > > _____________________________________________________ > FFmpeg issue tracker<[email protected]> > <https://roundup.ffmpeg.org/roundup/ffmpeg/issue1470> > _____________________________________________________ > > > bbc-ingex-archive-mxf.patch > > > Index: libavformat/utils.c > =================================================================== > --- libavformat/utils.c (revision 21408) > +++ libavformat/utils.c (working copy) > @@ -722,6 +722,7 @@ > case CODEC_ID_MJPEGB: > case CODEC_ID_LJPEG: > case CODEC_ID_RAWVIDEO: > + case CODEC_ID_V210: > case CODEC_ID_DVVIDEO: > case CODEC_ID_HUFFYUV: > case CODEC_ID_FFVHUFF: This can be applied right now. > Index: libavformat/mxfdec.c > =================================================================== > --- libavformat/mxfdec.c (revision 21408) > +++ libavformat/mxfdec.c (working copy) > @@ -91,11 +91,13 @@ > UID essence_container_ul; > UID essence_codec_ul; > AVRational sample_rate; > + AVRational audio_sampling_rate; > AVRational aspect_ratio; > int width; > int height; > int channels; > int bits_per_sample; > + int component_depth; > UID *sub_descriptors_refs; > int sub_descriptors_count; > int linked_track_id; > @@ -574,8 +576,8 @@ > descriptor->aspect_ratio.den = get_be32(pb); > break; > case 0x3D03: > - descriptor->sample_rate.num = get_be32(pb); > - descriptor->sample_rate.den = get_be32(pb); > + descriptor->audio_sampling_rate.num = get_be32(pb); > + descriptor->audio_sampling_rate.den = get_be32(pb); Ok, in a separate patch. > [...] > > @@ -801,31 +812,54 @@ > st->codec->codec_id = container_ul->id; > st->codec->width = descriptor->width; > st->codec->height = descriptor->height; > - st->codec->bits_per_coded_sample = descriptor->bits_per_sample; > /* Uncompressed */ > st->need_parsing = AVSTREAM_PARSE_HEADERS; > + if (st->codec->codec_id == CODEC_ID_RAWVIDEO || > st->codec->codec_id == CODEC_ID_V210) { > + if (descriptor->aspect_ratio.num> 0&& > descriptor->aspect_ratio.den> 0) { > + if (descriptor->sample_rate.num == 25&& > descriptor->sample_rate.den == 1) { > + if (descriptor->aspect_ratio.num == 4&& > descriptor->aspect_ratio.den == 3) > + st->codec->sample_aspect_ratio = > (AVRational){59, 54}; > + else if (descriptor->aspect_ratio.num == 16&& > descriptor->aspect_ratio.den == 9) > + st->codec->sample_aspect_ratio = > (AVRational){118, 81}; > + } > + else if (descriptor->sample_rate.num == 30000&& > descriptor->sample_rate.den == 1001) { > + if (descriptor->aspect_ratio.num == 4&& > descriptor->aspect_ratio.den == 3) > + st->codec->sample_aspect_ratio = > (AVRational){10, 11}; > + else if (descriptor->aspect_ratio.num == 16&& > descriptor->aspect_ratio.den == 9) > + st->codec->sample_aspect_ratio = > (AVRational){40, 33}; > + } I believe these sample aspect ratios are wrong for HD and wrong for SD without the clean aperture specified. FFmpeg is digital only so I recommend using the digital sample aspect ratio (DAR/PAR == width/height) since the clean aperture cannot really be specified currently. Patch welcome :) > + } > + if (st->codec->codec_id == CODEC_ID_V210 || > descriptor->component_depth == 10) { > + st->codec->codec_id = CODEC_ID_V210; > + st->codec->bits_per_raw_sample = 10; > + st->codec->pix_fmt = PIX_FMT_YUV422P16LE; Can we really have depth == 10 and codec_id != V210 ? The V210 bitstream is really specific regarding the packing of samples. > [...] > /* TODO: implement CODEC_ID_RAWAUDIO */ > if (st->codec->codec_id == CODEC_ID_PCM_S16LE) { > - if (descriptor->bits_per_sample == 24) > + if (descriptor->bits_per_sample> 24) > + st->codec->codec_id = CODEC_ID_PCM_S32LE; > + else if (descriptor->bits_per_sample> 16) Well <= 32 is more correct. If > 32 we need CODEC_ID_PCM_S64LE I guess :/ > [...] > > } > - if (st->codec->codec_type != CODEC_TYPE_DATA&& > (*essence_container_ul)[15]> 0x01) { > + if (st->codec->codec_type != CODEC_TYPE_DATA&& > ((*essence_container_ul)[15]& 0x03)> 0x01) { > av_log(mxf->fc, AV_LOG_WARNING, "only frame wrapped mappings > are correctly supported\n"); > st->need_parsing = AVSTREAM_PARSE_FULL; > } Was this rule made applicable for all essence containers ? I know the check is not ideal in the first place :/ _____________________________________________________ FFmpeg issue tracker <[email protected]> <https://roundup.ffmpeg.org/roundup/ffmpeg/issue1470> _____________________________________________________
