[vlc-devel] [vlc-commits] codec: don't drop discontinue blocks

Ilkka Ollakka ileoo at videolan.org
Tue Oct 13 23:12:13 CEST 2015


On Tue, Oct 13, 2015 at 11:50:01PM +0300, Rémi Denis-Courmont wrote:
> Le 2015-10-13 23:33, Ilkka Ollakka a écrit :
> >On Tue, Oct 13, 2015 at 10:24:44AM +0300, Rémi Denis-Courmont wrote:
> >>Le 2015-10-13 08:20, git at videolan.org a écrit :
> >>>vlc | branch: master | Ilkka Ollakka <ileoo at videolan.org> | Fri Sep
> >>>18 15:45:33 2015 +0300| [5e36cb2c1485830cf152c05767301a6732b08297] |
> >>>committer: Ilkka Ollakka

> >>>codec: don't drop discontinue blocks

> >>The patch does not do what it says. Especially flushing on discontinuity
> >>is
> >>obviously bad. Please revert.

> >Hi,

> >Patches are not reverted for futher commenting. Do you see other issue
> >with this patch than avcodec_flush_buffers in avcodec?

> You don't want to flush on discontinuity. In many cases, the discontinuity
> is due to packet loss. Flushing would drop all pending buffers, making the
> problem much worse than it really is. In some cases, the demuxer does not
> know whether the discontinuity is due to packet loss, or due to a new stream
> with a different timeline - ditto.

I agree with that, those codec changes can easily look like they would
add that flushing, but actually they just modified it so that it
wouldn't drop packets with that disco-flag like it does with corrupt
flag. Previous flow is that with disco-flag it will be dropped and codec
is reset, after this patch-set only corrupt-blocks are dropped and
otherwise codecs should behave close like before.

but yes, maybe in avcodec case etc we shouldn't flush buffers. Most
codecs go into 'get new sync point' with those date_Set(,0) changes,
like they would go previously. But yes, most likely those are
unnecessary.

> Maybe somtimes you know that the discontinuity is due to the start of a
> completely new timeline, and further maybe, you really want to discard all
> pending buffers when that happens (why?). But you cannot "reserve" the
> existing discontinuity flag just for that purpose. And as far as I know, the
> reset PCR control takes care of this case already.

Yes I agree that we shouldn't reinvent PCR control in disco flag,
purpose just merely to clarify what disco-flag means and to prevent
blocks being dropped if they happen to be disconnect and not corrupted.

So is it that discontinuity flag is some old relic that shouldn't be
even around anymore or was it meant to be used for something different
all together than signal that there could be packets missing between
this and previous block?

-- 
Ilkka Ollakka
It takes two to tell the truth: one to speak and one to hear.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20151014/ccc37090/attachment.sig>


More information about the vlc-devel mailing list