diff mbox

[FFmpeg-devel] dash encoder. Correct generated manifest for MPEG-DASH MPD Validator and calculate current bandwidth for eath chunk. Now works correctly with dash.sj

Message ID 2105862.8yzaTEHBEC@kid
State Superseded
Headers show

Commit Message

Ligverd Haer Sept. 5, 2016, 10:47 a.m. UTC
Create valide manifest.
Calculate current bandwidth for eath chunk.
Now works correctly with dash.sj

Comments

Carl Eugen Hoyos Sept. 6, 2016, 10:26 a.m. UTC | #1
2016-09-05 12:47 GMT+02:00 Ligverd Haer <ligverd@r46.ru>:
> Create valide manifest.
> Calculate current bandwidth for eath chunk.
> Now works correctly with dash.sj

> -        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\"
> segmentAlignment=\"true\" bitstreamSwitching=\"true\"");
> -        if (c->max_frame_rate.num && !c->ambiguous_frame_rate)
> -            avio_printf(out, " %s=\"%d/%d\"", (av_cmp_q(c->min_frame_rate,
> c->max_frame_rate) < 0) ? "maxFrameRate" : "frameRate",
> c->max_frame_rate.num, c->max_frame_rate.den);
> -        avio_printf(out, ">\n");
> +        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\"
> segmentAlignment=\"true\" bitstreamSwitching=\"true\">\n");

This hunk is not ok because it is needlessly difficult to understand
what you are changing:
Leave the first ("AdaptionSet") and last ("\n") line unchanged -
unless I misread the patch.

Carl Eugen
Ligverd Haer Sept. 6, 2016, 10:56 a.m. UTC | #2
В письме от вторник, 6 сентября 2016 г. 12:26:49 MSK пользователь Carl Eugen

Attributes

profiles
width
height
sar
frameRate
audioSamplingRate
mimeType
segmentProfiles
codecs
maximumSAPPeriod
startWithSAP
maxPlayoutRate
codingDependency
scanType

Common attributes for AdaptationSet and Representation shall either be in one 
of the elements but not in both.

in particular, the attribute frameRate is duplicated in the node 
Representation

> > -        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\"
> > segmentAlignment=\"true\" bitstreamSwitching=\"true\"");
> > -        if (c->max_frame_rate.num && !c->ambiguous_frame_rate)
> > -            avio_printf(out, " %s=\"%d/%d\"",
> > (av_cmp_q(c->min_frame_rate,
> > c->max_frame_rate) < 0) ? "maxFrameRate" : "frameRate",
> > c->max_frame_rate.num, c->max_frame_rate.den);
> > -        avio_printf(out, ">\n");
> > +        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\"
> > segmentAlignment=\"true\" bitstreamSwitching=\"true\">\n");
> 
> This hunk is not ok because it is needlessly difficult to understand
> what you are changing:
> Leave the first ("AdaptionSet") and last ("\n") line unchanged -
> unless I misread the patch.
Carl Eugen Hoyos Sept. 6, 2016, 11:02 a.m. UTC | #3
2016-09-06 12:56 GMT+02:00 Ligverd Haer <ligverd@r46.ru>:
> in particular, the attribute frameRate is duplicated in the node
> Representation

If this is the case, remove the offending line, do not
change the adjacent lines to make your intention easier
to understand.

Please do not top-post here, Carl Eugen
Ligverd Haer Sept. 6, 2016, 11:05 a.m. UTC | #4
В письме от вторник, 6 сентября 2016 г. 13:02:42 MSK пользователь Carl Eugen Hoyos написал:
> If this is the case, remove the offending line, do not
> change the adjacent lines to make your intention easier
> to understand.

Well, if that's clearer

     if (c->has_video) {
        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\" segmentAlignment=\"true\" bitstreamSwitching=\"true\"");
-        if (c->max_frame_rate.num && !c->ambiguous_frame_rate)
-            avio_printf(out, " %s=\"%d/%d\"", (av_cmp_q(c->min_frame_rate, c->max_frame_rate) < 0) ? "maxFrameRate" : "frameRate", c->max_frame_rate.num, c->max_frame_rate.den);
         avio_printf(out, ">\n");
Carl Eugen Hoyos Sept. 6, 2016, 11:09 a.m. UTC | #5
2016-09-06 13:05 GMT+02:00 Ligverd Haer <ligverd@r46.ru>:
> В письме от вторник, 6 сентября 2016 г. 13:02:42 MSK пользователь Carl Eugen Hoyos написал:

>> If this is the case, remove the offending line, do not

>> change the adjacent lines to make your intention easier

>> to understand.

>

> Well, if that's clearer


If you think it is not clearer, please ignore my suggestion!

Carl Eugen
Reuben Martin Sept. 8, 2016, 3:08 a.m. UTC | #6
On Tuesday, September 6, 2016 1:56:57 PM CDT Ligverd Haer wrote:
> В письме от вторник, 6 сентября 2016 г. 12:26:49 MSK пользователь Carl Eugen
> 
> Attributes
> 
> profiles
> width
> height
> sar
> frameRate
> audioSamplingRate
> mimeType
> segmentProfiles
> codecs
> maximumSAPPeriod
> startWithSAP
> maxPlayoutRate
> codingDependency
> scanType
> 
> Common attributes for AdaptationSet and Representation shall either be in
> one of the elements but not in both.
> 
> in particular, the attribute frameRate is duplicated in the node
> Representation

There is a bug report outstanding for this:
https://trac.ffmpeg.org/ticket/5087

-Reuben
Ligverd Haer Sept. 8, 2016, 6:58 a.m. UTC | #7
> > Common attributes for AdaptationSet and Representation shall either be in
> > one of the elements but not in both.
> > 
> > in particular, the attribute frameRate is duplicated in the node
> > Representation
> 
> There is a bug report outstanding for this:
> https://trac.ffmpeg.org/ticket/5087

Thank you Reuben, i posted in https://trac.ffmpeg.org/ticket/5087
Michael Niedermayer Sept. 8, 2016, 12:35 p.m. UTC | #8
On Thu, Sep 08, 2016 at 09:58:02AM +0300, Ligverd Haer wrote:
> > > Common attributes for AdaptationSet and Representation shall either be in
> > > one of the elements but not in both.
> > > 
> > > in particular, the attribute frameRate is duplicated in the node
> > > Representation
> > 
> > There is a bug report outstanding for this:
> > https://trac.ffmpeg.org/ticket/5087
> 
> Thank you Reuben, i posted in https://trac.ffmpeg.org/ticket/5087

patches should be sent to the mailing list, patches on trac tend to
be missed and forgotten

[...]
Ligverd Haer Sept. 8, 2016, 1:34 p.m. UTC | #9
В письме от четверг, 8 сентября 2016 г. 14:35:27 MSK пользователь Michael Niedermayer написал:
 
> patches should be sent to the mailing list, patches on trac tend to
> be missed and forgotten

Don't want to clog the ffmpeg-devel mailing list.
I made a pull request on github, I was sent to ffmpeg-devel mailing list, I sent a patch to the ffmpeg-devel mailing list, mene redirected to trac.ffmpeg.org

I agree the patch is not critical, and may be not worthy of attention.
In any case I tried. :(
Carl Eugen Hoyos Sept. 8, 2016, 1:41 p.m. UTC | #10
2016-09-08 15:34 GMT+02:00 Ligverd Haer <ligverd@r46.ru>:
> В письме от четверг, 8 сентября 2016 г. 14:35:27 MSK пользователь Michael Niedermayer написал:
>
>> patches should be sent to the mailing list, patches on trac tend to
>> be missed and forgotten
>
> Don't want to clog the ffmpeg-devel mailing list.

Please send your updated patch as attachment to a
reply in this mailing list thread,

Please tell us if documentation pointed you to github or trac to
post patches, we try hard to have patch submission (only) on
this mailing list.

Carl Eugen
diff mbox

Patch

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 519f9c4..06b4e60 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -498,18 +498,15 @@  static int write_manifest(AVFormatContext *s, int final)
         OutputStream *os = &c->streams[0];
         int start_index = FFMAX(os->nb_segments - c->window_size, 0);
         int64_t start_time = av_rescale_q(os->segments[start_index]->time, s->streams[0]->time_base, AV_TIME_BASE_Q);
-        avio_printf(out, "\t<Period start=\"");
+        avio_printf(out, "\t<Period%s start=\"",final?"":" id=\"0\"");
         write_time(out, start_time);
         avio_printf(out, "\">\n");
     } else {
-        avio_printf(out, "\t<Period start=\"PT0.0S\">\n");
+        avio_printf(out, "\t<Period%s start=\"PT0.0S\">\n",final?"":" id=\"0\"");
     }
 
     if (c->has_video) {
-        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\" segmentAlignment=\"true\" bitstreamSwitching=\"true\"");
-        if (c->max_frame_rate.num && !c->ambiguous_frame_rate)
-            avio_printf(out, " %s=\"%d/%d\"", (av_cmp_q(c->min_frame_rate, c->max_frame_rate) < 0) ? "maxFrameRate" : "frameRate", c->max_frame_rate.num, c->max_frame_rate.den);
-        avio_printf(out, ">\n");
+        avio_printf(out, "\t\t<AdaptationSet contentType=\"video\" segmentAlignment=\"true\" bitstreamSwitching=\"true\">\n");
 
         for (i = 0; i < s->nb_streams; i++) {
             AVStream *st = s->streams[i];
@@ -598,11 +595,15 @@  static int dash_write_header(AVFormatContext *s)
         AVDictionary *opts = NULL;
         char filename[1024];
 
-        os->bit_rate = s->streams[i]->codecpar->bit_rate;
+        os->bit_rate = s->streams[i]->codecpar->bit_rate ? s->streams[i]->codecpar->bit_rate :  s->bit_rate;
+
         if (os->bit_rate) {
             snprintf(os->bandwidth_str, sizeof(os->bandwidth_str),
                      " bandwidth=\"%d\"", os->bit_rate);
         } else {
+            snprintf(os->bandwidth_str, sizeof(os->bandwidth_str),
+                     " bandwidth=\"%d\"", 0);
+
             int level = s->strict_std_compliance >= FF_COMPLIANCE_STRICT ?
                         AV_LOG_ERROR : AV_LOG_WARNING;
             av_log(s, level, "No bit rate set for stream %d\n", i);
@@ -858,6 +859,10 @@  static int dash_flush(AVFormatContext *s, int final, int stream)
             if (ret < 0)
                 break;
         }
+
+        os->bit_rate = (int)( (float)range_length*8 / ((float)(os->max_pts - os->start_pts) / 10000) );
+        snprintf(os->bandwidth_str, sizeof(os->bandwidth_str)," bandwidth=\"%i\"", os->bit_rate);
+
         add_segment(os, filename, os->start_pts, os->max_pts - os->start_pts, start_pos, range_length, index_length);
         av_log(s, AV_LOG_VERBOSE, "Representation %d media segment %d written to: %s\n", i, os->segment_index, full_path);
     }