On Fri, 27 Jan 2017 15:39:30 +0200 (EET) Martin Storsjö <[email protected]> wrote:
> On Fri, 27 Jan 2017, Peter Große wrote: > > > This patch is based on the stream assignment code in webmdashenc. > > This muxer isn't available in libav, so the reference doesn't really work > in context here. True, I thought I changed to something like "This patch is based on the stream assignment code of a webm dash muxer by Vignesh Venkatasubramanian". So far only a big part of the parse_adaption_set function is copied over, the rest is basically my own code. But it looks like, I'll to also make changes to that part (regarding your comments below) so I might not need the reference anymore?! > > > > > Additional changes: > > * Default to one AdaptationSet per stream > > > > Previously all mapped streams of a media type (video, audio) where > > assigned to a single AdaptationSet. Using the DASH live profile it is > > mandatory, that the segments of all representations are aligned, which is > > currently not enforced. This leads to problems when using video streams > > with different key frame intervals. So to play safe, default to one > > AdaptationSet per stream, unless overwritten by explicit assignment > > Is there any way to easily get it back to the original behaviour, e.g. > tell it to group all audio tracks together and all video tracks together, > without having to manually specify the stream numbers? > > > * Make sure all streams are assigned to exactly one AdaptationSet > > > > Signed-off-by: Peter Große <[email protected]> > > --- > > libavformat/dashenc.c | 193 > > ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 162 > > insertions(+), 31 deletions(-) > > > > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > > index 1b12d4f..c561ad1 100644 > > --- a/libavformat/dashenc.c > > +++ b/libavformat/dashenc.c > > @@ -24,6 +24,7 @@ > > #include <unistd.h> > > #endif > > > > +#include "libavutil/avutil.h" > > #include "libavutil/avstring.h" > > #include "libavutil/eval.h" > > #include "libavutil/intreadwrite.h" > > @@ -58,9 +59,15 @@ typedef struct Segment { > > int n; > > } Segment; > > > > +typedef struct AdaptationSet { > > + char id[10]; > > + enum AVMediaType media_type; > > + AVDictionary *metadata; > > +} AdaptationSet; > > + > > typedef struct OutputStream { > > AVFormatContext *ctx; > > - int ctx_inited; > > + int ctx_inited, as_idx; > > uint8_t iobuf[32768]; > > AVIOContext *out; > > int duration_written; > > @@ -79,6 +86,9 @@ typedef struct OutputStream { > > > > typedef struct DASHContext { > > const AVClass *class; /* Class for private options. */ > > + char *adaptation_sets; > > + AdaptationSet *as; > > + int nb_as; > > int window_size; > > int extra_window_size; > > int min_seg_duration; > > @@ -87,7 +97,7 @@ typedef struct DASHContext { > > int use_timeline; > > int single_file; > > OutputStream *streams; > > - int has_video, has_audio; > > + int has_video; > > int64_t last_duration; > > int64_t total_duration; > > char availability_start_time[100]; > > @@ -176,6 +186,16 @@ static void dash_free(AVFormatContext *s) > > { > > DASHContext *c = s->priv_data; > > int i, j; > > + > > + if (c->as) { > > + for (i = 0; i < c->nb_as; i++) { > > + if (&c->as[i].metadata) > > + av_dict_free(&c->as[i].metadata); > > + } > > + av_freep(&c->as); > > + c->nb_as = 0; > > + } > > + > > if (!c->streams) > > return; > > for (i = 0; i < s->nb_streams; i++) { > > @@ -436,12 +456,144 @@ static void format_date_now(char *buf, int size) > > } > > } > > > > +static int write_adaptation_set(AVFormatContext *s, AVIOContext *out, int > > as_index) +{ > > + DASHContext *c = s->priv_data; > > + AdaptationSet *as = &c->as[as_index]; > > + int i; > > + > > + avio_printf(out, "\t\t<AdaptationSet id=\"%s\" contentType=\"%s\" > > segmentAlignment=\"true\" bitstreamSwitching=\"true\">\n", > > + as->id, as->media_type == AVMEDIA_TYPE_VIDEO ? "video" : > > "audio"); + > > + for (i = 0; i < s->nb_streams; i++) { > > + OutputStream *os = &c->streams[i]; > > + > > + if (os->as_idx - 1 != as_index) > > + continue; > > + > > + if (as->media_type == AVMEDIA_TYPE_VIDEO) { > > + avio_printf(out, "\t\t\t<Representation id=\"%d\" > > mimeType=\"video/mp4\" codecs=\"%s\"%s width=\"%d\" height=\"%d\">\n", > > + i, os->codec_str, os->bandwidth_str, > > s->streams[i]->codecpar->width, s->streams[i]->codecpar->height); > > + } else { > > + avio_printf(out, "\t\t\t<Representation id=\"%d\" > > mimeType=\"audio/mp4\" codecs=\"%s\"%s audioSamplingRate=\"%d\">\n", > > + i, os->codec_str, os->bandwidth_str, > > s->streams[i]->codecpar->sample_rate); > > + avio_printf(out, "\t\t\t\t<AudioChannelConfiguration > > schemeIdUri=\"urn:mpeg:dash:23003:3:audio_channel_configuration:2011\" > > value=\"%d\" />\n", > > + s->streams[i]->codecpar->channels); > > + } > > + output_segment_list(os, out, c); > > + avio_printf(out, "\t\t\t</Representation>\n"); > > + } > > + avio_printf(out, "\t\t</AdaptationSet>\n"); > > + > > + return 0; > > +} > > + > > +static int parse_adaptation_sets(AVFormatContext *s) > > +{ > > + DASHContext *c = s->priv_data; > > + char *p = c->adaptation_sets; > > + char *q; > > + enum { new_set, parse_id, parsing_streams } state; > > + int i; > > + > > + /* default: one AdaptationSet for each stream */ > > + if (!p) { > > + void *mem = av_mallocz(sizeof(*c->as) * s->nb_streams); > > + if (!mem) > > + return AVERROR(ENOMEM); > > + c->as = mem; > > + c->nb_as = s->nb_streams; > > + > > + for (i = 0; i < s->nb_streams; i++) { > > + AdaptationSet *as = &c->as[i]; > > + OutputStream *os = &c->streams[i]; > > + snprintf(as->id, sizeof(as->id), "%d", i); > > + as->metadata = NULL; > > + as->media_type = s->streams[i]->codecpar->codec_type; > > + os->as_idx = i + 1; > > + } > > + return 0; > > + } > > + > > + /* syntax id=0,streams=0,1,2 id=1,streams=3,4 and so on */ > > + state = new_set; > > + while (p < c->adaptation_sets + strlen(c->adaptation_sets)) { > > Isn't this equivalent to "while (*p) {"? Right. > > + if (*p == ' ') { > > + continue; > > If this ever happens, wouldn't you end up in an infinite loop? Also right. > > + } else if (state == new_set && !strncmp(p, "id=", 3)) { > > Instead of strncmp, you can also use av_strstart to avoid having to > hardcode the number of chars. ok. > > + AdaptationSet *as; > > + void *mem = av_realloc(c->as, sizeof(*c->as) * (c->nb_as + 1)); > > + if (!mem) > > + return AVERROR(ENOMEM); > > + c->as = mem; > > + ++c->nb_as; > > + > > + as = &c->as[c->nb_as - 1]; > > + as->metadata = NULL; > > maybe memset(as, 0, sizeof(*as))? Then you'd reduce the risk of missing to > initialize a new field here, if AdaptationSet is expanded later. Should do the trick, yes. > > > + as->media_type = AVMEDIA_TYPE_UNKNOWN; > > + > > + p += 3; // consume "id=" > > av_strstart can avoid hardcoding this number here ok. > > + q = as->id; > > + while (*p != ',') *q++ = *p++; > > Keep normal indentation for the assignment. > > The loop lacks a condition for *p == '\0', so unless there's a trailing > comma somewhere, you'll run out of bounds and crash. > > It also lacks a check for the buffer size of as->id. > > Something like this could also work: > > n = strcspn(p, ","); > snprintf(as->id, sizeof(as->id), "%*s", n, p); > p += n; > if (*p) > p++; Right. > > + *q = 0; > > + p++; > > + state = parse_id; > > + } else if (state == parse_id && !strncmp(p, "streams=", 8)) { > > + p += 8; // consume "streams=" > > + state = parsing_streams; > > + } else if (state == parsing_streams) { > > + struct AdaptationSet *as = &c->as[c->nb_as - 1]; > > + OutputStream *os; > > + char *stream_identifier; > > + > > + q = p; > > + while (*q != '\0' && *q != ',' && *q != ' ') q++; > > + stream_identifier = av_strndup(p, q - p); > > This should also be doable with strcspn(p, " ,"). And copy it into a local > stack buffer (e.g. with snprintf as above, to get bounds checking of that > buffer for free) instead of a strdup of a couple-bytes string. ok. > > + > > + i = strtol(stream_identifier, NULL, 10); > > + if (i < 0 || i >= s->nb_streams) { > > + av_log(s, AV_LOG_ERROR, "Selected stream \"%s\" not > > found!\n", stream_identifier); > > If you hit this case, you leak stream_identifier (unless you make it a > local stack buffer as I suggested). Will fix that. > > + return -1; > > Please return proper error codes, e.g. AVERROR(EINVAL) here. ok. Thanks for the comments ;) Regards Peter
pgp6DABEuoeqE.pgp
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
