[FFmpeg-devel] [PATCH] Add SUP/PGS subtitle demuxer

[Reimar Döffinger]

On Sun, Aug 31, 2014 at 12:00:13PM +0200, wm4 wrote:
> On Sun, 31 Aug 2014 11:50:36 +0200
> Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> 
> > On Sun, Aug 31, 2014 at 11:39:16AM +0200, wm4 wrote:
> > > I got the semantics of avio_read wrong: short reads on EOF are in
> > > fact allowed and don't return an error. Hopefully this is correct now...
> > 
> > What I _really_ dislike about these approaches is that we hard-code EOF
> > behaviour in every single demuxer.
> > So some will return partial packets some won't.
> > And if we wanted to make that behaviour configurable we'd have to add
> > options to every single one instead of changing the code in one single
> > place.
> > Not to mention all the other optimizations, just look how much e.g.
> > av_get_packet actually does!
> > Why do you absolutely want to handle this manually via avio_read?
> > You can just use av_get_packet + av_append_packet, then you only need
> > to check the size once, and you also don't have to set "pkt->pos"
> > manually and you still don't have to do seekback.
> 
> I wasn't aware of av_get/append_packet() (sorry if it was mentioned
> before and I ignored it)

I mentioned it but was too lazy to find out the exact function names.
So I'll take part of the blame if you missed it :)

> - it looks nice. I'll try to use it and send a
> patch again later. It's a bit not-nice that it reallocs the packet, but
> there's probably no way it matters in this case at all.

I agree. _If_ it mattered at some point it would be possible to have a variant
that specifies an initial allocation size - for many cases it would be
possible to reasonably estimate the final packet size.
Though especially for this case that would be just overkill anyway and I think going
for "as simple as possible" is best.