[FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available
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, &pkt, 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(&pkt); return ret; -- 1.7.10.4 ___ 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
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
Re: [FFmpeg-devel] [PATCH v2] lavf/avienc: Use palette side data packet if available
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
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
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
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