Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Mats Peterson]

On 02/28/2016 02:15 AM, Reimar Döffinger wrote:

+for (i = 0; i < pkt->side_data_elems; i++) {
+if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
+pal = pkt->side_data[i].data;
+break;
+}
+}


av_frame_get_side_data


No frames available in a muxer (as far as I know). But thanks anyway...


av_packet_get_side_data then.


Yes, that's more elegant of course. I just "copied" a snippet from 
nutenc.c in the meantime...










-uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
-avio_wb32(pb, v<<8);
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wl32(pb, v & 0xff);


You'll probably still need the if to use the one or the avio_w...
depending on format.
Unless it's possible to change the embedded format to match
the side data one.



Don't know what you mean by "Probably still need the if".


Forget this and the rest, I simply misread the patch diff.


No problem, Reimar.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Reimar Döffinger]

On 28.02.2016, at 01:13, Mats Peterson  wrote:

> On 02/28/2016 12:34 AM, Reimar Döffinger wrote:
> 
>> On Sat, Feb 27, 2016 at 10:55:13PM +0100, Mats Peterson wrote:
>>> -if (!avist->hdr_pal_done) {
>>> -int64_t cur_offset = avio_tell(pb);
>>> -avio_seek(pb, avist->pal_offset, SEEK_SET);
>>> -for (i = 0; i < pal_size; i++) {
>>> -uint32_t v = AV_RL32(data + size - 4*pal_size + 
>>> 4*i);
>>> -avio_wl32(pb, v & 0xff);
>>> -}
>>> -avio_seek(pb, cur_offset, SEEK_SET);
>>> -avist->hdr_pal_done++;
>>> -}
>> 
>> Hadn't seen this before.
>> In principle it is a bit unfortunate as it means that streaming via
>> stdout is not possible.
>> It should at least be suppressed when seeking the output stream
>> is not possible.
> 
> Right. Good point.
> 
>> 
>> 
>>> +for (i = 0; i < pkt->side_data_elems; i++) {
>>> +if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
>>> +pal = pkt->side_data[i].data;
>>> +break;
>>> +}
>>> +}
>> 
>> av_frame_get_side_data
> 
> No frames available in a muxer (as far as I know). But thanks anyway...

av_packet_get_side_data then.

> 
>> 
>>> -uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
>>> -avio_wb32(pb, v<<8);
>>> +uint32_t v = AV_RL32(pal + 4*i);
>>> +avio_wl32(pb, v & 0xff);
>> 
>> You'll probably still need the if to use the one or the avio_w...
>> depending on format.
>> Unless it's possible to change the embedded format to match
>> the side data one.
>> 
> 
> Don't know what you mean by "Probably still need the if".

Forget this and the rest, I simply misread the patch diff.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Mats Peterson]

On 02/28/2016 01:13 AM, Mats Peterson wrote:

On 02/28/2016 12:34 AM, Reimar Döffinger wrote:


On Sat, Feb 27, 2016 at 10:55:13PM +0100, Mats Peterson wrote:

-if (!avist->hdr_pal_done) {
-int64_t cur_offset = avio_tell(pb);
-avio_seek(pb, avist->pal_offset, SEEK_SET);
-for (i = 0; i < pal_size; i++) {
-uint32_t v = AV_RL32(data + size -
4*pal_size + 4*i);
-avio_wl32(pb, v & 0xff);
-}
-avio_seek(pb, cur_offset, SEEK_SET);
-avist->hdr_pal_done++;
-}


Hadn't seen this before.
In principle it is a bit unfortunate as it means that streaming via
stdout is not possible.
It should at least be suppressed when seeking the output stream
is not possible.


Right. Good point.





+for (i = 0; i < pkt->side_data_elems; i++) {
+if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
+pal = pkt->side_data[i].data;
+break;
+}
+}


av_frame_get_side_data


No frames available in a muxer (as far as I know). But thanks anyway...




-uint32_t v = AV_RL32(data + size - 4*pal_size +
4*i);
-avio_wb32(pb, v<<8);
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wl32(pb, v & 0xff);


You'll probably still need the if to use the one or the avio_w...
depending on format.
Unless it's possible to change the embedded format to match
the side data one.



Don't know what you mean by "Probably still need the if".



+for (i = 0; i < pal_size; i++) {
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wb32(pb, v<<8);


Why is the header format different? That seems a bit weird.
___


The palette uses a RGBQUAD (BGRA) structure for the entries in the
palette after the BITMAPINFOHEADER, but it uses an AVIPALCHANGE
structure (RGBA) for those 'xxpc' chunks. It may seem a bit confusing,


Well, the AVIPALCHANGE structure uses a PALETTEENTRY structure, that's 
the core of it, and it uses RGB order, with a flag byte afterwards.


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Mats Peterson]

On 02/28/2016 12:34 AM, Reimar Döffinger wrote:


On Sat, Feb 27, 2016 at 10:55:13PM +0100, Mats Peterson wrote:

-if (!avist->hdr_pal_done) {
-int64_t cur_offset = avio_tell(pb);
-avio_seek(pb, avist->pal_offset, SEEK_SET);
-for (i = 0; i < pal_size; i++) {
-uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
-avio_wl32(pb, v & 0xff);
-}
-avio_seek(pb, cur_offset, SEEK_SET);
-avist->hdr_pal_done++;
-}


Hadn't seen this before.
In principle it is a bit unfortunate as it means that streaming via
stdout is not possible.
It should at least be suppressed when seeking the output stream
is not possible.


Right. Good point.





+for (i = 0; i < pkt->side_data_elems; i++) {
+if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
+pal = pkt->side_data[i].data;
+break;
+}
+}


av_frame_get_side_data


No frames available in a muxer (as far as I know). But thanks anyway...




-uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
-avio_wb32(pb, v<<8);
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wl32(pb, v & 0xff);


You'll probably still need the if to use the one or the avio_w...
depending on format.
Unless it's possible to change the embedded format to match
the side data one.



Don't know what you mean by "Probably still need the if".



+for (i = 0; i < pal_size; i++) {
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wb32(pb, v<<8);


Why is the header format different? That seems a bit weird.
___


The palette uses a RGBQUAD (BGRA) structure for the entries in the 
palette after the BITMAPINFOHEADER, but it uses an AVIPALCHANGE 
structure (RGBA) for those 'xxpc' chunks. It may seem a bit confusing, 
but I'm doing everything by the book. It's all in the AVI specification.


I should probably never have said "I'm new at this". I know the specs 
alright.


I still got no answer to why the packet data is sometimes zero bytes 
when doing stream copy, but I suppose Michael knows.


Mats

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Reimar Döffinger]

On Sat, Feb 27, 2016 at 10:55:13PM +0100, Mats Peterson wrote:
> -if (!avist->hdr_pal_done) {
> -int64_t cur_offset = avio_tell(pb);
> -avio_seek(pb, avist->pal_offset, SEEK_SET);
> -for (i = 0; i < pal_size; i++) {
> -uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
> -avio_wl32(pb, v & 0xff);
> -}
> -avio_seek(pb, cur_offset, SEEK_SET);
> -avist->hdr_pal_done++;
> -}

Hadn't seen this before.
In principle it is a bit unfortunate as it means that streaming via
stdout is not possible.
It should at least be suppressed when seeking the output stream
is not possible.


> +for (i = 0; i < pkt->side_data_elems; i++) {
> +if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
> +pal = pkt->side_data[i].data;
> +break;
> +}
> +}

av_frame_get_side_data

> -uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
> -avio_wb32(pb, v<<8);
> +uint32_t v = AV_RL32(pal + 4*i);
> +avio_wl32(pb, v & 0xff);

You'll probably still need the if to use the one or the avio_w...
depending on format.
Unless it's possible to change the embedded format to match
the side data one.

> +for (i = 0; i < pal_size; i++) {
> +uint32_t v = AV_RL32(pal + 4*i);
> +avio_wb32(pb, v<<8);

Why is the header format different? That seems a bit weird.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available

[Mats Peterson]

Still as experimental as the first one. Seems that the packets are empty 
(zero bytes) now and then when doing stream copy. It would be 
interesting to know why, Michael. I'm new at this.


--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From a803d1d4995b371e357f3e280ee37019d77c254e Mon Sep 17 00:00:00 2001
From: Mats Peterson 
Date: Sat, 27 Feb 2016 22:53:37 +0100
Subject: [PATCH v2] lavf/avienc: Use palette side data packet if available

---
 libavformat/avienc.c |   61 ++
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index ca505f4..1b76728 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -668,39 +668,52 @@ static int avi_write_packet(AVFormatContext *s, AVPacket *pkt)
 if ((ret = write_skip_frames(s, stream_index, pkt->dts)) < 0)
 return ret;
 
-if (enc->codec_id == AV_CODEC_ID_RAWVIDEO && enc->codec_tag == 0) {
+if (enc->codec_id == AV_CODEC_ID_RAWVIDEO && enc->codec_tag == 0 && size) {
 int64_t bpc = enc->bits_per_coded_sample != 15 ? enc->bits_per_coded_sample : 16;
 int expected_stride = ((enc->width * bpc + 31) >> 5)*4;
+const uint8_t *pal = NULL;
+int pal_size = 1 << enc->bits_per_coded_sample;
+int i;
 
 ret = ff_reshuffle_raw_rgb(s, , enc, expected_stride);
 if (ret < 0)
 return ret;
-if (ret) {
-if (ret == CONTAINS_PAL) {
-int pc_tag, i;
-int pal_size = 1 << enc->bits_per_coded_sample;
-if (!avist->hdr_pal_done) {
-int64_t cur_offset = avio_tell(pb);
-avio_seek(pb, avist->pal_offset, SEEK_SET);
-for (i = 0; i < pal_size; i++) {
-uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
-avio_wl32(pb, v & 0xff);
-}
-avio_seek(pb, cur_offset, SEEK_SET);
-avist->hdr_pal_done++;
-}
-avi_stream2fourcc(tag, stream_index, enc->codec_type);
-tag[2] = 'p'; tag[3] = 'c';
-pc_tag = ff_start_tag(pb, tag);
-avio_w8(pb, 0);
-avio_w8(pb, pal_size & 0xFF);
-avio_wl16(pb, 0); // reserved
+
+for (i = 0; i < pkt->side_data_elems; i++) {
+if (pkt->side_data[i].type == AV_PKT_DATA_PALETTE) {
+pal = pkt->side_data[i].data;
+break;
+}
+}
+if (!pal && ret == CONTAINS_PAL)
+pal = data + size - AVPALETTE_SIZE;
+
+if (pal) {
+int pc_tag, i;
+if (!avist->hdr_pal_done) {
+int64_t cur_offset = avio_tell(pb);
+avio_seek(pb, avist->pal_offset, SEEK_SET);
 for (i = 0; i < pal_size; i++) {
-uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
-avio_wb32(pb, v<<8);
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wl32(pb, v & 0xff);
 }
-ff_end_tag(pb, pc_tag);
+avio_seek(pb, cur_offset, SEEK_SET);
+avist->hdr_pal_done++;
+}
+avi_stream2fourcc(tag, stream_index, enc->codec_type);
+tag[2] = 'p'; tag[3] = 'c';
+pc_tag = ff_start_tag(pb, tag);
+avio_w8(pb, 0);
+avio_w8(pb, pal_size & 0xFF);
+avio_wl16(pb, 0); // reserved
+for (i = 0; i < pal_size; i++) {
+uint32_t v = AV_RL32(pal + 4*i);
+avio_wb32(pb, v<<8);
 }
+ff_end_tag(pb, pc_tag);
+}
+
+if (ret) {
 ret = avi_write_packet_internal(s, pkt);
 av_packet_free();
 return ret;
-- 
1.7.10.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel