| Message ID | 20170920030028.3098-2-atomnuker@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 20 Sep 2017 04:00:28 +0100 Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > + if (mdm.has_gamma || mdm.has_primaries) { > + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); > + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); > + } > + Use assignment instead of memcpy, duh.
On 9/20/2017 3:18 PM, wm4 wrote: > On Wed, 20 Sep 2017 04:00:28 +0100 > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > >> + if (mdm.has_gamma || mdm.has_primaries) { >> + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); >> + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); >> + } >> + > > Use assignment instead of memcpy, duh. He shouldn't be using AVMasteringDisplayMetadata on stack to begin with, so no. It's bad enough sizeof() has to be used at all already.
On Wed, 20 Sep 2017 04:00:28 +0100 Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > + if (mdm.has_gamma || mdm.has_primaries) { > + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); > + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); > + } > + Use assignment? (Not sure if my previous reply to this was sent by my retarded mail client, what a fucking POS.)
On Wed, 20 Sep 2017 15:22:42 -0300 James Almer <jamrial@gmail.com> wrote: > On 9/20/2017 3:18 PM, wm4 wrote: > > On Wed, 20 Sep 2017 04:00:28 +0100 > > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > >> + if (mdm.has_gamma || mdm.has_primaries) { > >> + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); > >> + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); > >> + } > >> + > > > > Use assignment instead of memcpy, duh. > > He shouldn't be using AVMasteringDisplayMetadata on stack to begin with, > so no. > > It's bad enough sizeof() has to be used at all already. Again you're just creating new problems out of thin air. We don't realistically support mismatched libs, even if in theory we do. I wish we could stop wasting time on things that don't matter at all.
On Wed, Sep 20, 2017 at 08:32:59PM +0200, wm4 wrote: > On Wed, 20 Sep 2017 15:22:42 -0300 > James Almer <jamrial@gmail.com> wrote: > > > On 9/20/2017 3:18 PM, wm4 wrote: > > > On Wed, 20 Sep 2017 04:00:28 +0100 > > > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > > > >> + if (mdm.has_gamma || mdm.has_primaries) { > > >> + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); > > >> + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); > > >> + } > > >> + > > > > > > Use assignment instead of memcpy, duh. > > > > He shouldn't be using AVMasteringDisplayMetadata on stack to begin with, > > so no. > > > > It's bad enough sizeof() has to be used at all already. > > Again you're just creating new problems out of thin air. We don't > realistically support mismatched libs, even if in theory we do. I wish > we could stop wasting time on things that don't matter at all. For past releases there was no dependace between the libs except what was implied by their version numbers and that was tested before release. Its also expected by some if not all distributions. And is generally the standard More so we define versions based on semver, in our API docs. Which too requires specific (version dependant) package/lib compatibility. From this it results that if sizeof can ever change without a major bump than sizeof cannot be easily used outside the lib its comming from. It can be used with precautions but that can becomes ugly quick This does matter also in reality as its very easy by installing a new package on a system to pull in a update for a dependant lib. While the other libs wont be updated as there is no need to for installing that package. Thanks [...]
On Wed, 20 Sep 2017 22:00:34 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Sep 20, 2017 at 08:32:59PM +0200, wm4 wrote: > > On Wed, 20 Sep 2017 15:22:42 -0300 > > James Almer <jamrial@gmail.com> wrote: > > > > > On 9/20/2017 3:18 PM, wm4 wrote: > > > > On Wed, 20 Sep 2017 04:00:28 +0100 > > > > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > > > > > >> + if (mdm.has_gamma || mdm.has_primaries) { > > > >> + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); > > > >> + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); > > > >> + } > > > >> + > > > > > > > > Use assignment instead of memcpy, duh. > > > > > > He shouldn't be using AVMasteringDisplayMetadata on stack to begin with, > > > so no. > > > > > > It's bad enough sizeof() has to be used at all already. > > > > Again you're just creating new problems out of thin air. We don't > > realistically support mismatched libs, even if in theory we do. I wish > > we could stop wasting time on things that don't matter at all. > > For past releases there was no dependace between the libs except > what was implied by their version numbers and that was tested before > release. Its also expected by some if not all distributions. And is > generally the standard > > More so we define versions based on semver, in our API docs. > Which too requires specific (version dependant) package/lib compatibility. > > From this it results that if sizeof can ever change without a major bump > than sizeof cannot be easily used outside the lib its comming from. > It can be used with precautions but that can becomes ugly quick > > This does matter also in reality as its very easy by installing a > new package on a system to pull in a update for a dependant lib. While > the other libs wont be updated as there is no need to for installing > that package. I know all this, I'm just saying it's stupid to generate extra work for "supporting" this.
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c index 0d6612ccca..b7d9ded89c 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -25,6 +25,7 @@ #include "libavutil/bprint.h" #include "libavutil/imgutils.h" #include "libavutil/stereo3d.h" +#include "libavutil/mastering_display_metadata.h" #include "avcodec.h" #include "bytestream.h" @@ -1163,10 +1164,15 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, AVFrame *p, AVPacket *avpkt) { AVDictionary **metadatap = NULL; + AVMasteringDisplayMetadata mdm; uint32_t tag, length; int decode_next_dat = 0; int ret; + mdm.has_primaries = 0; + mdm.has_luminance = 0; + mdm.has_gamma = 0; + for (;;) { length = bytestream2_get_bytes_left(&s->gb); if (length <= 0) { @@ -1287,6 +1293,41 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, goto fail; break; } + case MKTAG('c', 'H', 'R', 'M'): { + mdm.white_point[0].num = bytestream2_get_be32(&s->gb); + mdm.white_point[0].den = 100000; + mdm.white_point[1].num = bytestream2_get_be32(&s->gb); + mdm.white_point[1].den = 100000; + + /* Red primaries */ + mdm.display_primaries[0][0].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[0][0].den = 100000; + mdm.display_primaries[0][1].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[0][1].den = 100000; + + /* Green primaries */ + mdm.display_primaries[1][0].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[1][0].den = 100000; + mdm.display_primaries[1][1].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[1][1].den = 100000; + + /* Blue primaries */ + mdm.display_primaries[2][0].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[2][0].den = 100000; + mdm.display_primaries[2][1].num = bytestream2_get_be32(&s->gb); + mdm.display_primaries[2][1].den = 100000; + + mdm.has_primaries = 1; + bytestream2_skip(&s->gb, 4); /* crc */ + break; + } + case MKTAG('g', 'A', 'M', 'A'): { + mdm.gamma.num = bytestream2_get_be32(&s->gb); + mdm.gamma.den = 100000; + mdm.has_gamma = 1; + bytestream2_skip(&s->gb, 4); /* crc */ + break; + } case MKTAG('I', 'E', 'N', 'D'): if (!(s->pic_state & PNG_ALLIMAGE)) av_log(avctx, AV_LOG_ERROR, "IEND without all image\n"); @@ -1305,6 +1346,11 @@ skip_tag: } exit_loop: + if (mdm.has_gamma || mdm.has_primaries) { + AVMasteringDisplayMetadata *new_mdm = av_mastering_display_metadata_create_side_data(p); + memcpy(new_mdm, &mdm, sizeof(AVMasteringDisplayMetadata)); + } + if (avctx->codec_id == AV_CODEC_ID_PNG && avctx->skip_frame == AVDISCARD_ALL) { return 0;
Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> --- libavcodec/pngdec.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)