video_output: avoid dropping special pictures
Don't drop the first picture and still pictures if they are late. They may be the only pictures available for a while and we need something to display.
Simplify the render_now
flag: always true except when we use a new picture that has its own timing (unless it's the first, we want it right away).
This effectively duplicates some of the current usage of the picture_t::b_force
flag, which shouldn't have to decide rendering policies.
Merge request reports
Activity
changed milestone to %4.0
added Component::Core: Video output label
added MRStatus::Reviewable label
There are 2 things at play here
- dropping the first picture
- showing the first picture earlier than it should
IMO the latter shouldn't be there. In practice it's probably rare to have the video ready to show before the audio so it wouldn't make much difference. But if the audio is meant to start "without video" (usually meaning black) then it's wrong.
For the former it could be a problem on slow decoding scenario. If the first picture arrives late, we could always wait for the next non-late. But there may never be a non-late picture, or not for a while. In this case the VLC approach has been to show at least the first frame and maybe at some point it gets better, rather than showing black until it gets better (ie never).
So this patch doesn't change this choice. It narrows down the places where changes would be needed to change this. I have some patches after that to narrow this even more (right now the force flag is set on the first decoded picture). In the end that choice should be only in the video output.
IMO the latter shouldn't be there. In practice it's probably rare to have the video ready to show before the audio so it wouldn't make much difference. But if the audio is meant to start "without video" (usually meaning black) then it's wrong.
I'm not sure it's accurate.
In this sample file, audio PTS is basically earlier than video PTS most of the time, and it's like most of my client streams, and I'm not sure I saw a lot of file where it's not the case. Maybe this could be verified?
Just for the record, the behaviour "without video" will be platform-dependant. On iOS it will be the application background. On other platforms the previous frame might still be copied on the screen. On video callback we typically don't send a black frame.
Indeed, I confused the meaning of the beginning of the sentence, but then I don't understand the meaning of the rest of the sentence:
IMO the latter shouldn't be there. In practice it's probably rare to have the video ready to show before the audio so it wouldn't make much difference. But if the audio is meant to start "without video" (usually meaning black) then it's wrong.
Not displaying the first frame earlier will make a difference since video timestamps might be scheduled a bit later than audio, or do I confuse something else?
If the previous track is not cleared when the previous file is unloaded and even present when another file starts. It's a bug. The fact we want to show the first frame only when it's supposed to may exhibit it. That doesn't mean it should not be fixed. And that doesn't mean it should stop us from showing the first frame only when it should.
In practice the display is restarted between files so I don't think the previous file can remain there. There might be a window without nothing drawn (different behavior per platform). But that's already the case now.
added MRStatus::InReview label and removed MRStatus::Reviewable label
mentioned in merge request !641
1509 1510 // next date we need to display again the current picture 1510 1511 date_refresh = sys->displayed.date + VOUT_REDISPLAY_DELAY - render_delay; 1511 1512 refresh = date_refresh <= system_now; 1512 render_now = refresh; IMO it's removing some confusion. Right now you have to understand when
render_now
becomes true or false there.When using the "redisplay" mechanism either we want to redisplay in this call and refresh is true, we don't need to set render_now to true, as it's the default value. When we don't redisplay (refresh = false) we don't need to set render_now because we are not going to do the rendering (it can't be the first and we didn't drop the current frame in this call). We will return VLC_EGENERIC.
And it probably makes even more sense to always set it to
true
onredisplay
anyway. The PTS of the picture to redisplay is old and we will likely display it right away as there's nothing to wait for. (maybe there's some trick to shift the PTS for redisplay, it doesn't change the fact it's old and can be shown ASAP, waiting between prepare and display is bad)
added MRStatus::Stale label and removed MRStatus::InReview label
added MRStatus::InReview label and removed MRStatus::Stale label
added MRStatus::NotCompliant label and removed MRStatus::InReview label