[vlc-devel] [PATCH 0/6] Map some of the missing avcodec IDs

[Daniel Verkamp]

On Tue, Jan 21, 2014 at 5:45 PM, Daniel Verkamp <daniel at drv.nu> wrote:
> On Sun, Jan 12, 2014 at 8:58 AM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
>> On 12 Jan, Daniel Verkamp wrote :
>>> On Sat, Jan 11, 2014 at 3:27 AM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
>>> > On 10 Jan, Daniel Verkamp wrote :
>>> >> For now, I'm just sending the ones that I got to that actually work;
>>> >
> [...]
>>> > Did you try to force --no-avcodec-dr on those?
>>>
>>> I tried a few of the broken codecs with it now, but it seems to make
>>> no difference. The majority of these are 8-bit palette video formats;
>>> fixing that would allow a lot more codecs to work (even e.g. Smacker
>>> video, which is already mapped but doesn't work due to using a
>>> palette).
>>
>> Shouldn't it be simple to fix, by letting swscale module do the job?
>
> That sounds reasonable, but I don't understand VLC's video pipeline
> enough to implement that...
>
> It seems like palette passing should be doable, since it works in some
> cases (e.g. classic Windows clock.avi with palette works with VLC's
> native AVI demux + avcodec msrle).
>
> Thanks,
> -- Daniel Verkamp

I have done some more digging into the video palette decoding issue,
and it looks pretty clear that VLC is just totally ignoring
codec-provided palettes (as opposed to container-provided palettes
from e.g. AVI) from libavcodec.

Try this (totally bogus, for debugging only) patch with a Smacker
file, for instance:
http://samples.mplayerhq.hu/game-formats/smacker/smk-deen/credits.smk

You should see some decoded video with the wrong colors (as opposed to
all black).

diff --git a/modules/video_chroma/swscale.c b/modules/video_chroma/swscale.c
index 1751131..9e1f481 100644
--- a/modules/video_chroma/swscale.c
+++ b/modules/video_chroma/swscale.c
@@ -559,6 +559,12 @@ static void Convert( filter_t *p_filter, struct
SwsContext *ctx,
         if( p_filter->fmt_in.video.p_palette )
             memcpy( palette, p_filter->fmt_in.video.p_palette->palette,
                     __MIN( sizeof(video_palette_t), AVPALETTE_SIZE ) );
+        else
+            for (int i = 0; i < AVPALETTE_SIZE; i += 4)
+            {
+                palette[i + 0] = palette[i + 1] = palette[i + 2] = i;
+                palette[i + 3] = 0xFF;
+            }
         src[1] = palette;
         src_stride[1] = 4;
     }

The existence of this "if (palette)" is strange anyway - it's an error
condition if the palette is not provided, and the palette is just left
black, resulting in the current behavior.

I think the correct place to grab the palette is during
ffmpeg_CopyPicture, but my quick hack doesn't seem to work, and the
palette gets lost somewhere along the line - the "no palette" case is
still hit later on in swscale:

diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index 83c5954..55511bf 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -891,6 +891,18 @@ static void ffmpeg_CopyPicture( decoder_t *p_dec,
         uint8_t *p_dst, *p_src;
         int i_src_stride, i_dst_stride;

+        if( p_ff_pic->palette_has_changed )
+        {
+            if( !p_dec->fmt_out.video.p_palette )
+            {
+                p_dec->fmt_out.video.p_palette = calloc( 1,
sizeof(video_palette_t) );
+                p_dec->fmt_out.video.p_palette->i_entries = 256;
+            }
+            memcpy( p_dec->fmt_out.video.p_palette->palette,
p_ff_pic->data[1], AVPALETTE_SIZE ); // TODO: are these in the same
format/size?
+        }
+
         for( i_plane = 0; i_plane < p_pic->i_planes; i_plane++ )
         {
             p_src  = p_ff_pic->data[i_plane];

Please feel free to pick this up and fix it; I probably won't have the
time to look into it further for a little while.

Thanks,
-- Daniel Verkamp