[FFmpeg-devel] [PATCH] Fix incorrect decoding of DXSA subtitles (DivX subtitle with alpha)

[Reimar Döffinger]

On Tue, Apr 26, 2011 at 10:48:46AM +0200, Alexandre Colucci wrote:
> > On Thu, Apr 21, 2011 at 11:49:09AM +0200, Alexandre Colucci wrote:
> >> This patch fixes the DXSA subtitles decoding. See screenshots for before and after the patch.
> >> If the fourcc is DXSA, there are extra 4 bytes before the compressed data. The specifications are not public but the format is supported by DivX Player.
> > 
> > I do not like depending on the tag.
> 
> 
> That is what the official reference implementation does.
> 
> 
> > I assume there is nothing in the data itself to distinguish
> > between the formats?
> 
> There isn't, that's why the reference implementation looks at the codec tag.
> 
> > If not I think it might be better to add a new CODEC_ID.
> 
> A trivial 4 byte difference does not really justify a new CODEC_ID, especially since that would require changing several files including those in libavformat and may break existing code.
> 
> > But either way, the code should just be
> > |= *buf++ << 24;
> > which will work if you do not run the loop above that case.
> > Or use the same loop for both case, e.g. (pseudo-code):
> > for (i = 0; i < sub->rects[0]->nb_colors; i++) {
> >    uint32_t alpha = i ? 0xff000000 : 0;
> >    if (tag == DXSA)
> >        alpha = *buf++ << 24;
> >    ((uint32_t*)sub->rects[0]->pict.data[1])[i] |= alpha;
> > }
> 
> Attached is the revised patch (functionally equivalent to the previous one) with only one loop.

It still looks a bit ugly to me but I don't have any
convincingly better way.
So it is fine by me, any objections by anyone else?