On 03/18/2014 07:12 AM, wm4 wrote: > On Mon, 17 Mar 2014 20:59:37 -0600 > John Stebbins <[email protected]> wrote: > >> --- >> libavformat/movenc.c | 95 >> +++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 91 insertions(+), 4 deletions(-) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index 2ae3475..3f1e65b 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -308,7 +308,9 @@ static int mov_write_esds_tag(AVIOContext *pb, MOVTrack >> *track) // Basic >> >> // the following fields is made of 6 bits to identify the streamtype (4 >> for video, 5 for audio) >> // plus 1 bit to indicate upstream and 1 bit set to 1 (reserved) >> - if (track->enc->codec_type == AVMEDIA_TYPE_AUDIO) >> + if (track->enc->codec_id == AV_CODEC_ID_DVD_SUBTITLE) >> + avio_w8(pb, (0x38 << 2) | 1); // flags (= NeroSubpicStream) >> + else if (track->enc->codec_type == AVMEDIA_TYPE_AUDIO) >> avio_w8(pb, 0x15); // flags (= Audiostream) >> else >> avio_w8(pb, 0x11); // flags (= Visualstream) >> @@ -759,6 +761,7 @@ static int mp4_get_codec_tag(AVFormatContext *s, >> MOVTrack *track) >> else if (track->enc->codec_id == AV_CODEC_ID_VC1) tag = >> MKTAG('v','c','-','1'); >> else if (track->enc->codec_type == AVMEDIA_TYPE_VIDEO) tag = >> MKTAG('m','p','4','v'); >> else if (track->enc->codec_type == AVMEDIA_TYPE_AUDIO) tag = >> MKTAG('m','p','4','a'); >> + else if (track->enc->codec_id == AV_CODEC_ID_DVD_SUBTITLE) tag = >> MKTAG('m','p','4','s'); >> >> return tag; >> } >> @@ -981,6 +984,21 @@ static int mov_write_subtitle_tag(AVIOContext *pb, >> MOVTrack *track) >> return update_size(pb, pos); >> } >> >> +static int mov_write_subpic_tag(AVIOContext *pb, MOVTrack *track) >> +{ >> + int64_t pos = avio_tell(pb); >> + >> + avio_wb32(pb, 0); /* size */ >> + avio_wl32(pb, track->tag); // store it byteswapped >> + avio_wb32(pb, 0); /* Reserved */ >> + avio_wb16(pb, 0); /* Reserved */ >> + avio_wb16(pb, 1); /* Data-reference index */ >> + >> + mov_write_esds_tag(pb, track); >> + >> + return update_size(pb, pos); >> +} >> + >> static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track) >> { >> AVRational sar; >> @@ -1116,6 +1134,8 @@ static int mov_write_stsd_tag(AVIOContext *pb, >> MOVTrack *track) >> mov_write_video_tag(pb, track); >> else if (track->enc->codec_type == AVMEDIA_TYPE_AUDIO) >> mov_write_audio_tag(pb, track); >> + else if (track->enc->codec_id == AV_CODEC_ID_DVD_SUBTITLE) >> + mov_write_subpic_tag(pb, track); >> else if (track->enc->codec_type == AVMEDIA_TYPE_SUBTITLE) >> mov_write_subtitle_tag(pb, track); >> else if (track->enc->codec_tag == MKTAG('r','t','p',' ')) >> @@ -1307,6 +1327,7 @@ static int mov_write_hdlr_tag(AVIOContext *pb, >> MOVTrack *track) >> descr = "SoundHandler"; >> } else if (track->enc->codec_type == AVMEDIA_TYPE_SUBTITLE) { >> if (track->tag == MKTAG('t','x','3','g')) hdlr_type = "sbtl"; >> + if (track->tag == MKTAG('m','p','4','s')) hdlr_type = "subp"; >> else hdlr_type = "text"; >> descr = "SubtitleHandler"; >> } else if (track->enc->codec_tag == MKTAG('r','t','p',' ')) { >> @@ -3175,6 +3196,68 @@ static void mov_free(AVFormatContext *s) >> av_freep(&mov->tracks); >> } >> >> +static uint32_t rgb_to_yuv(uint32_t rgb) >> +{ >> + uint8_t r, g, b; >> + int y, cb, cr; >> + >> + r = (rgb >> 16) & 0xFF; >> + g = (rgb >> 8) & 0xFF; >> + b = (rgb ) & 0xFF; >> + >> + y = av_clip_uint8( 16. + 0.257 * r + 0.504 * g + 0.098 * b); >> + cb = av_clip_uint8(128. - 0.148 * r - 0.291 * g + 0.439 * b); >> + cr = av_clip_uint8(128. + 0.439 * r - 0.368 * g - 0.071 * b); >> + >> + return (y << 16) | (cr << 8) | cb; >> +} >> + >> +static int mov_rewrite_dvd_sub_extradata(MOVTrack *track, AVStream *st) > Maybe give it a different name, as function for reading isom.c uses the > same name. This could be confusing. (Maybe the reader function's naming > wasn't descriptive enough either.)
Ok. How about mov_create_dvd_sub_decoder_specific_info. Kind of long, but
more descriptive of what it does.
>> +{
>> + int i, width, height;
>> + int have_palette, have_size;
>> + uint32_t palette[16];
>> + char *cur;
>> +
>> + if (!st->codec->extradata || !st->codec->extradata_size)
>> + return AVERROR_INVALIDDATA;
> Shouldn't that be allowed? Sometimes you don't have the extradata.
> OTOH, I wouldn't mind to reject muxing if extradata is missing just for
> the sake to prevent production of "problematic" files.
Ok. We can allow this. It is a good point that some source files may not have
this extradata.
>> + cur = st->codec->extradata;
>> + while (*cur) {
>> + if (strncmp("palette:", cur, 8) == 0) {
>> + int i;
>> + char *p = cur + 8;
>> + for (i = 0; i < 16; i++) {
>> + palette[i] = rgb_to_yuv(strtoul(p, &p, 16));
> Maybe also use sscanf here for minimal error checking? It's a bit
> strange that it will accept anything for the palette, but is stricter
> with the size. (Anyway, this is just a nit.)
Ok.
>> + while (*p == ',' || av_isspace(*p))
>> + p++;
>> + }
>> + have_palette = 1;
>> + } else if (!strncmp("size:", cur, 5)) {
>> + if (sscanf(cur + 5, "%dx%d", &width, &height) != 2) {
>> + return AVERROR_INVALIDDATA;
>> + }
>> + have_size = 1;
>> + }
>> + if (have_palette && have_size)
>> + break;
>> + cur += strcspn(cur, "\n\r");
>> + cur += strspn(cur, "\n\r");
>> + }
>> + if (!have_palette || !have_size)
>> + return AVERROR_INVALIDDATA;
> Again, I think it's not totally sure whether maybe missing palette/size
> should be allowed or not.
Ok
>> +
>> + track->vos_data = av_malloc(16*4);
> Not checking return value.
OK
>> + for (i = 0; i < 16; i++) {
>> + AV_WB32(track->vos_data + i * 4, palette[i]);
>> + }
>> + track->vos_len = 16 * 4;
>> + st->codec->width = width;
>> + st->codec->height = track->height = height;
> track->width is not set?
There is no MOVTrack.width member.
>> + return 0;
>> +}
>> +
>> static int mov_write_header(AVFormatContext *s)
>> {
>> AVIOContext *pb = s->pb;
>> @@ -3331,9 +3414,13 @@ static int mov_write_header(AVFormatContext *s)
>>
>> /* copy extradata if it exists */
>> if (st->codec->extradata_size) {
>> - track->vos_len = st->codec->extradata_size;
>> - track->vos_data = av_malloc(track->vos_len);
>> - memcpy(track->vos_data, st->codec->extradata, track->vos_len);
>> + if (st->codec->codec_id == AV_CODEC_ID_DVD_SUBTITLE)
>> + mov_rewrite_dvd_sub_extradata(track, st);
>> + else {
>> + track->vos_len = st->codec->extradata_size;
>> + track->vos_data = av_malloc(track->vos_len);
>> + memcpy(track->vos_data, st->codec->extradata,
>> track->vos_len);
>> + }
>> }
>> }
>>
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
--
John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
