| Message ID | 20200220162601.943-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Quoting James Almer (2020-02-20 17:26:00) > Signed-off-by: James Almer <jamrial@gmail.com> Commit message is now misleading since it will only enable prft if it's not disabled. > --- > Now it can be overriden if you explicitly set write_prft to 0. > > libavformat/dashenc.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > index a52cbc9113..7032adc84d 100644 > --- a/libavformat/dashenc.c > +++ b/libavformat/dashenc.c > @@ -1394,6 +1394,12 @@ static int dash_init(AVFormatContext *s) > c->frag_type = FRAG_TYPE_EVERY_FRAME; > } > > + if (c->write_prft < 0) { > + c->write_prft = c->ldash; nit: !!, in case ldash becomes something else than a bool in the future > + if (c->ldash) > + av_log(s, AV_LOG_INFO, "Enabling Producer Reference Time element for Low Latency mode\n"); I'd say this should be VERBOSE, since a normal run with no unexpected events should produce no log output. Otherwise LGTM.
On 2/24/2020 6:54 AM, Anton Khirnov wrote: > Quoting James Almer (2020-02-20 17:26:00) >> Signed-off-by: James Almer <jamrial@gmail.com> > > Commit message is now misleading since it will only enable prft if it's > not disabled. Sorry, i pushed this during the weekend. And, true. It's still attempting but technically not always... Which makes me realize i should mention this undocumented behavior in the doxy. >> --- >> Now it can be overriden if you explicitly set write_prft to 0. >> >> libavformat/dashenc.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c >> index a52cbc9113..7032adc84d 100644 >> --- a/libavformat/dashenc.c >> +++ b/libavformat/dashenc.c >> @@ -1394,6 +1394,12 @@ static int dash_init(AVFormatContext *s) >> c->frag_type = FRAG_TYPE_EVERY_FRAME; >> } >> >> + if (c->write_prft < 0) { >> + c->write_prft = c->ldash; > > nit: !!, in case ldash becomes something else than a bool in the future The chances for that are pretty slim, since turning a bool into an int would be an API break (true/false would stop working from the command line, afaik). But i can change it anyway. > >> + if (c->ldash) >> + av_log(s, AV_LOG_INFO, "Enabling Producer Reference Time element for Low Latency mode\n"); > > I'd say this should be VERBOSE, since a normal run with no unexpected > events should produce no log output. Sure, will change in a new commit. > > Otherwise LGTM. > Thanks, and apologies for not waiting a bit more.
Quoting James Almer (2020-02-26 01:28:48) > On 2/24/2020 6:54 AM, Anton Khirnov wrote: > > Quoting James Almer (2020-02-20 17:26:00) > >> Signed-off-by: James Almer <jamrial@gmail.com> > > > > Commit message is now misleading since it will only enable prft if it's > > not disabled. > > Sorry, i pushed this during the weekend. And, true. It's still > attempting but technically not always... > > Which makes me realize i should mention this undocumented behavior in > the doxy. > > >> --- > >> Now it can be overriden if you explicitly set write_prft to 0. > >> > >> libavformat/dashenc.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > >> index a52cbc9113..7032adc84d 100644 > >> --- a/libavformat/dashenc.c > >> +++ b/libavformat/dashenc.c > >> @@ -1394,6 +1394,12 @@ static int dash_init(AVFormatContext *s) > >> c->frag_type = FRAG_TYPE_EVERY_FRAME; > >> } > >> > >> + if (c->write_prft < 0) { > >> + c->write_prft = c->ldash; > > > > nit: !!, in case ldash becomes something else than a bool in the future > > The chances for that are pretty slim, since turning a bool into an int > would be an API break (true/false would stop working from the command > line, afaik). But i can change it anyway. I mean someone could do exactly the thing you are doing here - use -1 for default/unset. > > > > Otherwise LGTM. > > > > Thanks, and apologies for not waiting a bit more. No problem, I should have looked more closely before replying. They were minor comments anyway.
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c index a52cbc9113..7032adc84d 100644 --- a/libavformat/dashenc.c +++ b/libavformat/dashenc.c @@ -1394,6 +1394,12 @@ static int dash_init(AVFormatContext *s) c->frag_type = FRAG_TYPE_EVERY_FRAME; } + if (c->write_prft < 0) { + c->write_prft = c->ldash; + if (c->ldash) + av_log(s, AV_LOG_INFO, "Enabling Producer Reference Time element for Low Latency mode\n"); + } + if (c->write_prft && !c->utc_timing_url) { av_log(s, AV_LOG_WARNING, "Producer Reference Time element option will be ignored as utc_timing_url is not set\n"); c->write_prft = 0; @@ -2352,7 +2358,7 @@ static const AVOption options[] = { { "lhls", "Enable Low-latency HLS(Experimental). Adds #EXT-X-PREFETCH tag with current segment's URI", OFFSET(lhls), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E }, { "ldash", "Enable Low-latency dash. Constrains the value of a few elements", OFFSET(ldash), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E }, { "master_m3u8_publish_rate", "Publish master playlist every after this many segment intervals", OFFSET(master_publish_rate), AV_OPT_TYPE_INT, {.i64 = 0}, 0, UINT_MAX, E}, - { "write_prft", "Write producer reference time element", OFFSET(write_prft), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, E}, + { "write_prft", "Write producer reference time element", OFFSET(write_prft), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, E}, { "mpd_profile", "Set profiles. Elements and values used in the manifest may be constrained by them", OFFSET(profile), AV_OPT_TYPE_FLAGS, {.i64 = MPD_PROFILE_DASH }, 0, UINT_MAX, E, "mpd_profile"}, { "dash", "MPEG-DASH ISO Base media file format live profile", 0, AV_OPT_TYPE_CONST, {.i64 = MPD_PROFILE_DASH }, 0, UINT_MAX, E, "mpd_profile"}, { "dvb_dash", "DVB-DASH profile", 0, AV_OPT_TYPE_CONST, {.i64 = MPD_PROFILE_DVB }, 0, UINT_MAX, E, "mpd_profile"},
Signed-off-by: James Almer <jamrial@gmail.com> --- Now it can be overriden if you explicitly set write_prft to 0. libavformat/dashenc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)