[FFmpeg-devel] [PATCH 6/8] avcodec/h264_redundant_pps_bsf: implement a AVBSFContext.flush() callback

[Michael Niedermayer]

On Wed, Aug 01, 2018 at 11:20:46PM +0100, Mark Thompson wrote:
> On 28/07/18 22:59, Michael Niedermayer wrote:
> > On Fri, Jul 27, 2018 at 11:57:47AM -0300, James Almer wrote:
> >> Signed-off-by: James Almer <jamrial at gmail.com>
> >> ---
> >> I'm not 100% sure this is correct. I also don't know if the CBS contexts need
> >> to be fully reinitialized or not in this scenario. Because if so, then every
> >> bsf using cbs will require a flush() callback as well.
> >>
> >>  libavcodec/h264_redundant_pps_bsf.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> > 
> > Do we have a fate test for this ?
> 
> No - I don't have a public file it applies to.  (I can share an example privately.)
> 
> > Also from a quick look, isnt this filter lacking checks ?
> > it removes PPS but i see no check that these PPS differ only in the qp
> > related parameters. Also doesnt this filter miss that there can be multiple
> > PPS with different id#
> 
> The problematic streams repeatedly overwrite PPS id 0 with different parameters, and that's what we're fixing up here - using a stream like that in any case with global extradata fails on seeking or fragmenting.
> 

> > Maybe iam missing something and this is of course not about the
> > patch at hand but it looks like this is a quite specific filter that
> > cannot saftely be applied to h264 in general and if so this is not
> > obvious from the documentation.
> 
> The documentation says the filter "applies a specific fixup to some Blu-ray streams", which implies that it doesn't apply to anything else?  Feel free to add some additional text which clarifies it to your reading if you don't agree.

I think we interpret this differently.
"applies F to B" to me does not imply anything about "Not B". And naivly i
would have expected that it does not break "Not B".
Being able to "just" apply this filter to all h264 without the need to manually
first check each file if it is originating from blue-ray seems a realistic use
case. I mean it could be a bit incovenient if one had a large amount of diverse
input material. More so if not all is h264 ...
Obviously this was not what others had in mind 

I can suggest a patch to clarify the docs if there is no interrest in making
tha filter safe to "just" add

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180803/d78d66ee/attachment.sig>