Also, I think the decoder should consider the VBV delay as well, which would be initial_cpb_removal_delay incase of H264

But who should post it, the parser or the decoder?

This was completely covered in the past. A ffmpeg API change we missed ?

Even the VBV delay part was covered? Is there any approximate version where it was known to work?

Oops, in fact we only consider the reordering of b-frames. But the way it's implemented, there is no other delay information that can be used.

As of now:

 latency = gst_util_uint64_scale_ceil (
        (ffmpegdec->context->has_b_frames) * GST_SECOND, info->fps_d,
        info->fps_n);

So we are looking for a number of frames that we scale in millisecond using the framerate. Not sure that exist, but if we had a live stream with rate != 1, that match would not be correct. Let's ignore for now. Looking at the doc, I found the following to impact the latency:

if (thread_type == FF_THREAD_FRAME)
  frame_latency += thread_count

vbv_delay is deprecated, and not implemented in anything useful. It's also specific to MPEG2 Video. In H264, this encoder parameter does not affect the decoding delay.

There is delay, and video_delay, which I guess were supposed to be sine aggregated delay, but in fact they are only partially implemented (apedec, opusdec, mpegvideo_enc, some filters and gifdec). Nothing interesting really. video_delay is no implemented anywhere useful, it is also not documented.

So all in all, I think the correct formula would be:

  frame_latency = ffmpegdec->context->has_b_frames

  if (thread_type == FF_THREAD_FRAME)
    frame_latency += thread_count

  latency = gst_util_uint64_scale_ceil (frame_latency * GST_SECOND, info->fps_d,
        info->fps_n);

But, if the pipeline is live, we always set thread_type to FF_THREAD_SLICE to avoid the extra delays. So in the end, our formula is mainly the best we can do.

Now, has_b_frames is a confusing name, so there could be a bug in the implementation. It's name makes it look like a boolean, but it's a size:
  
  Size of the frame reordering buffer in the decoder.

I think you should trace that value to help further with this bug report.

(Oh, the other option, is that the framerate is unknown in this stream, in which case we can't figure-out the delay from a frame count)

1. I think the assumption is that vbv_delay is deprecated for H264 is not right. H264 has equivalent delay named HRD delay(Hypothetical reference decoder). The decoder is expected to remove the first frame from the VBV/HRD buffer only after initial_cpb_removal_delay has elapsed, thereby adding delay

2. Agree on the part that decoder's thread too to be used for computing delay

Now, whatever we do need to be aligned with ffmpeg code, if you think the ffmpeg decoder is introducing some delay related to vbv, then you need to wire-it up. The H264 decoder in ffmpeg does not set that context member. This is theoretical bug here.

On the other hand, the bug you report is that latency does not change when you change the number of b-frames. This is implemented. Also, it does not change if you change the number of threads, this is intentional, we use slice threading, which does not introduce latency (according to the doc, in fact there should be 1 line latency, but we are frame based in gst).

My guess is that you have no framerate to compute the latency. Would it be possible to report the caps used so we can confirm or not this.

Ping ?

Sorry for being away. I'm doing some more investigation on this. Will post my observations

Some key points are also captured in https://bugzilla.gnome.org/show_bug.cgi?id=773084 by Edward

"Due to that difference, tsdemux needs to report not the end-to-end latency (PTS - PCR received at the same time) but only the min/max latency it introduces itself. This boils down to the interleaving/packetization latency. The other downstream elements (parsers, queue, decoders, ...) will add min/max latency specific to those streams and end up with the ideal capture-to-display latency."

That's for tsdemux, so what's the problem with avvidec? :)

:) 

I guess all the latencies, whether B frame ordering or VBV/HRD delay should be reported by codecparsers than avvidec. Isn't it?

If so, we need to stop reporting latencies within the decoder

In that case, decoder should report only processing latency, which should be max one frame worth of delay for a any realtime decoder. This seems to get complicated, Or am I grossly wrong? :)

No, only the element buffering shall report latency. The action of re-ordering is what introduce latency in H264 decoders, and this is done by the decoder atm. For this reason, decoder are reporting it.

I have asked a question earlier. Let me rephrase. Do you have an example stream and C code that demonstrate the latency is incorrect. Can you share this so we can reproduce and focus on an issue ? This thread right now looks more like a forum discussion then a bug report.

Sorry for the long silence.

I have debugged this further and found out the following
1. has_b_frames is used for calculating reorder latency
2. This variable gets set only after first call to gst_ffmpegviddec_video_frame. Till then it is set to 0
3. We are reading this variable in set_format() during which it is not set. Which means we are reading it much before it is set
If I move the latency reporting part to handle_frame, this works fine. But is it the best place?

Steps to reproduce:
Sender:
gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! x264enc bframes=4 key-int-max=25 b-adapt=0 ! mpegtsmux ! udpsink host=10.0.90.1 port=8000

Receiver:
gst-launch-1.0 udpsrc port=8000 ! tsdemux ! h264parse ! avdec_h264  ! fakesink sync=1

14:43 < ndufresne> someone need to validate this, I guess it will be much easier when I get the per element latency tracing working
14:44 < ndufresne> you need to test that couple of H264 streams, with different profiles and specially different number of b-frames
14:44 < ndufresne> and then see if the latency match the required decoder depth, or if it's totally random
14:44 < ndufresne> in ffmpeg, if you enable frame threading, it also introduce 1 frame latency (no matter how many thread you pick)
14:45 < ndufresne> ^ but that could easily be our fault, our decoder is not thread and does not use the new read/write API

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-libav/issues/30.