[FFmpeg-devel] [PATCH] flac: add GIF image support
The FLAC specification requires GIF images to contain their number of
colors. While I can't find a specific reference to that effect, I'm
assuming that's why GIF images were previously unsupported. This was
implemented by just writing AVPALETTE_COUNT for paletted images.
---
Changelog | 1 +
libavformat/flacenc.c | 10 ++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/Changelog b/Changelog
index e8ee74775e..3ff783c61d 100644
--- a/Changelog
+++ b/Changelog
@@ -61,6 +61,7 @@ version :
- shear filter
- kirsch filter
- colortemperature filter
+- GIF support in FLAC
version 4.3:
diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index 1c983486aa..c156d13b14 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -151,11 +151,16 @@ static int flac_write_picture(struct AVFormatContext *s,
AVPacket *pkt)
avio_wb32(pb, st->codecpar->width);
avio_wb32(pb, st->codecpar->height);
+
if ((pixdesc = av_pix_fmt_desc_get(st->codecpar->format)))
avio_wb32(pb, av_get_bits_per_pixel(pixdesc));
else
avio_wb32(pb, 0);
-avio_wb32(pb, 0);
+
+if (st->codecpar->format == AV_PIX_FMT_PAL8)
+avio_wb32(pb, AVPALETTE_COUNT);
+else
+avio_wb32(pb, 0);
avio_wb32(pb, pkt->size);
avio_write(pb, pkt->data, pkt->size);
@@ -218,9 +223,6 @@ static int flac_init(struct AVFormatContext *s)
if (!(st->disposition & AV_DISPOSITION_ATTACHED_PIC)) {
av_log(s, AV_LOG_WARNING, "Video stream #%d is not an attached
picture. Ignoring\n", i);
continue;
-} else if (st->codecpar->codec_id == AV_CODEC_ID_GIF) {
-av_log(s, AV_LOG_ERROR, "GIF image support is not
implemented.\n");
-return AVERROR_PATCHWELCOME;
} else if (!c->write_header) {
av_log(s, AV_LOG_ERROR, "Can't write attached pictures without
a header.\n");
return AVERROR(EINVAL);
--
2.28.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] flac: add GIF image support
This patch no longer makes an (accidental) change to adjacent code and isn't included in the changelog. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] flac: add GIF image support
The FLAC specification requires GIF images to contain their number of
colors. While I can't find a specific reference to that effect, I'm
assuming that's why GIF images were previously unsupported. This was
implemented by just writing AVPALETTE_COUNT for paletted images.
---
libavformat/flacenc.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index 1c983486aa..c9834b7d93 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -155,7 +155,10 @@ static int flac_write_picture(struct AVFormatContext *s,
AVPacket *pkt)
avio_wb32(pb, av_get_bits_per_pixel(pixdesc));
else
avio_wb32(pb, 0);
-avio_wb32(pb, 0);
+if (st->codecpar->format == AV_PIX_FMT_PAL8)
+avio_wb32(pb, AVPALETTE_COUNT);
+else
+avio_wb32(pb, 0);
avio_wb32(pb, pkt->size);
avio_write(pb, pkt->data, pkt->size);
@@ -218,9 +221,6 @@ static int flac_init(struct AVFormatContext *s)
if (!(st->disposition & AV_DISPOSITION_ATTACHED_PIC)) {
av_log(s, AV_LOG_WARNING, "Video stream #%d is not an attached
picture. Ignoring\n", i);
continue;
-} else if (st->codecpar->codec_id == AV_CODEC_ID_GIF) {
-av_log(s, AV_LOG_ERROR, "GIF image support is not
implemented.\n");
-return AVERROR_PATCHWELCOME;
} else if (!c->write_header) {
av_log(s, AV_LOG_ERROR, "Can't write attached pictures without
a header.\n");
return AVERROR(EINVAL);
--
2.28.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3] flac: add GIF image support
The FLAC specification requires GIF images to contain their number of
colors. While I can't find a specific reference to that effect, I'm
assuming that's why GIF images were previously unsupported. This was
implemented by just writing AVPALETTE_COUNT for paletted images.
---
This version no longer includes a changelog entry or accidentally adds a
newline to existing code in flac_write_picture. This is identical to v2
except for fixing a mistake while submitting, sorry about that.
libavformat/flacenc.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index 1c983486aa..c9834b7d93 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -155,7 +155,10 @@ static int flac_write_picture(struct AVFormatContext *s,
AVPacket *pkt)
avio_wb32(pb, av_get_bits_per_pixel(pixdesc));
else
avio_wb32(pb, 0);
-avio_wb32(pb, 0);
+if (st->codecpar->format == AV_PIX_FMT_PAL8)
+avio_wb32(pb, AVPALETTE_COUNT);
+else
+avio_wb32(pb, 0);
avio_wb32(pb, pkt->size);
avio_write(pb, pkt->data, pkt->size);
@@ -218,9 +221,6 @@ static int flac_init(struct AVFormatContext *s)
if (!(st->disposition & AV_DISPOSITION_ATTACHED_PIC)) {
av_log(s, AV_LOG_WARNING, "Video stream #%d is not an attached
picture. Ignoring\n", i);
continue;
-} else if (st->codecpar->codec_id == AV_CODEC_ID_GIF) {
-av_log(s, AV_LOG_ERROR, "GIF image support is not
implemented.\n");
-return AVERROR_PATCHWELCOME;
} else if (!c->write_header) {
av_log(s, AV_LOG_ERROR, "Can't write attached pictures without
a header.\n");
return AVERROR(EINVAL);
--
2.28.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] flac: add GIF image support
That's true, but as far as I can tell FFmpeg assumes that all paletted images have 256 colors, and thus there isn't a way to get the exact number of colors in a GIF. I also doubt that this will cause problems in practice, it's somewhat strange that this field is even there. If I'm wrong on any of this, please let me know. On Sun, Jan 31, 2021 at 3:07 PM Derek Buitenhuis wrote: > On 29/01/2021 16:12, leo60228 wrote: > > +if (st->codecpar->format == AV_PIX_FMT_PAL8) > > +avio_wb32(pb, AVPALETTE_COUNT); > > +else > > +avio_wb32(pb, 0); > > Is this correct, though? > > GIFs encoded by things that are not libavcodec may have less than > AVPALETTE_COUNT entries/colors. > > > - Derek > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] flac: add GIF image support
After some further thought and discussion on the FFmpeg and Xiph.org IRC channels, the current behavior seems reasonable. To my knowledge, PAL8 is guaranteed to have exactly 256 colors. This is why, somewhat surprisingly, GIFs aren't decoded as PAL8, they're decoded as BGRA. This means that they get a 0 written. In addition, it was suggested that 0 could mean that the number of colors is unknown, which would be exactly the case in this scenario. These fields don't seem to be considered especially important, either. For example, the reference FLAC encoder always writes 24-bit color for GIF, despite having a comment acknowledging that this may be incorrect. On Sun, Jan 31, 2021 at 3:41 PM leo60228 wrote: > That's true, but as far as I can tell FFmpeg assumes that all paletted > images have 256 colors, and thus there isn't a way to get the exact number > of colors in a GIF. > I also doubt that this will cause problems in practice, it's somewhat > strange that this field is even there. > If I'm wrong on any of this, please let me know. > > On Sun, Jan 31, 2021 at 3:07 PM Derek Buitenhuis < > derek.buitenh...@gmail.com> wrote: > >> On 29/01/2021 16:12, leo60228 wrote: >> > +if (st->codecpar->format == AV_PIX_FMT_PAL8) >> > +avio_wb32(pb, AVPALETTE_COUNT); >> > +else >> > +avio_wb32(pb, 0); >> >> Is this correct, though? >> >> GIFs encoded by things that are not libavcodec may have less than >> AVPALETTE_COUNT entries/colors. >> >> >> - Derek >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
