On Sat, 17 Aug 2013, Luca Barbato wrote:
Prevent an assert and a segfault.
---
I'm not 100% happy about the fallback.
libavformat/movenc.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index ef03857..66751b6 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1239,11 +1239,12 @@ static int mov_write_hdlr_tag(AVIOContext *pb, MOVTrack
*track)
const char *hdlr, *descr = NULL, *hdlr_type = NULL;
int64_t pos = avio_tell(pb);
- if (!track) { /* no media --> data handler */
- hdlr = "dhlr";
- hdlr_type = "url ";
- descr = "DataHandler";
- } else {
+ /* no media --> data handler */
+ hdlr = "dhlr";
+ hdlr_type = "url ";
+ descr = "DataHandler";
+
+ if (track) {
Hmm, I'm not sure I understand what you gain from this part of the patch
hdlr = (track->mode == MODE_MOV) ? "mhlr" : "\0\0\0\0";
if (track->enc->codec_type == AVMEDIA_TYPE_VIDEO) {
hdlr_type = "vide";
@@ -1258,6 +1259,18 @@ static int mov_write_hdlr_tag(AVIOContext *pb, MOVTrack
*track)
} else if (track->enc->codec_tag == MKTAG('r','t','p',' ')) {
hdlr_type = "hint";
descr = "HintHandler";
+ } else if (track->enc->codec_tag == MKTAG('t','m','c','d')) {
+ hdlr_type = "tmcd";
+ descr = "TimeCodeHandler";
I guess this part would be ok
+ } else {
+ char tag_buf[32];
+ av_get_codec_tag_string(tag_buf, sizeof(tag_buf),
+ track->enc->codec_tag);
+
+ av_log(track->enc, AV_LOG_WARNING,
+ "Unknown hldr_type for %s / 0x%04X, writing dummy values\n",
+ tag_buf, track->enc->codec_tag);
+ return 0;
Possibly this as well
}
}
@@ -1640,7 +1653,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb,
MOVMuxContext *mov)
int version;
for (i = 0; i < mov->nb_streams; i++) {
- if (mov->tracks[i].entry > 0) {
+ if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
max_track_len_temp = av_rescale_rnd(mov->tracks[i].track_duration,
MOV_TIMESCALE,
mov->tracks[i].timescale,
Doesn't the last hunk make this unnecessary, at least for the data case?
@@ -3126,6 +3139,8 @@ static int mov_write_header(AVFormatContext *s)
}
} else if (st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE) {
track->timescale = st->codec->time_base.den;
+ } else if (st->codec->codec_type == AVMEDIA_TYPE_DATA) {
+ track->timescale = st->codec->time_base.den;
}
This hunk probably is good. Or should we make it a generic catch-all else
statement? We don't want tracks without a timescale initialized. Or should
we error out completely if trying to add an unsupported track?
With either of these in place, I guess the above hunk in
mov_write_mvhd_tag wouldn't be necessary?
In general, most of the changes probably are good, but I'd like to have
them split into individual steps with explanations for each of them to
understand their individual roles, e.g. "make sure timescale is
initialized, avoiding assert failures" or "handle use case X better"
(which is more of a feature than a bugfix).
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
