From: Nuo Mi <nuomi2021@gmail.com> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] 回复: [PATCH v1 09/19] avcodec/h274: add ff_h274_hash functions Date: Sat, 31 May 2025 09:53:40 +0800 Message-ID: <CAFXK13emh1RGQDh9QfRa4hEypbqUaNs8b=WXD=Q0XsOopJhR-Q@mail.gmail.com> (raw) In-Reply-To: <CAFXK13d4qY=75nbNqDVmh=rg_ACPpWLezfz5OpN=PkmJ7NTnfA@mail.gmail.com> On Sat, May 24, 2025 at 10:02 PM Nuo Mi <nuomi2021@gmail.com> wrote: > > > On Fri, May 9, 2025 at 2:37 AM Wu Jianhua <toqsxw@outlook.com> wrote: > >> Andreas Rheinhardt: >> > toqsxw@gmail.com: >> >> From: Wu Jianhua <toqsxw@outlook.com> >> >> >> >> Signed-off-by: Wu Jianhua <toqsxw@outlook.com> >> >> --- >> >> libavcodec/h274.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++ >> >> libavcodec/h274.h | 7 ++ >> >> 2 files changed, 170 insertions(+) >> >> >> >> diff --git a/libavcodec/h274.c b/libavcodec/h274.c >> >> index 5709200322..9edc705042 100644 >> >> --- a/libavcodec/h274.c >> >> +++ b/libavcodec/h274.c >> >> @@ -26,7 +26,11 @@ >> >> */ >> >> >> >> #include "libavutil/avassert.h" >> >> +#include "libavutil/bswap.h" >> >> +#include "libavutil/crc.h" >> >> #include "libavutil/imgutils.h" >> >> +#include "libavutil/md5.h" >> >> +#include "libavutil/mem.h" >> >> >> >> #include "h274.h" >> >> >> >> @@ -790,3 +794,162 @@ static const int8_t R64T[64][64] = { >> >> 17, -16, 15, -14, 13, -12, 11, -10, 9, -8, 7, -6, >> 4, -3, 2, -1, >> >> } >> >> }; >> >> + >> >> +static int verify_plane_md5(struct AVMD5 *ctx, >> >> + const uint8_t *src, const int w, const int h, const int stride, >> >> + const uint8_t *expected) >> >> +{ >> >> +#define MD5_SIZE 16 >> >> + uint8_t md5[MD5_SIZE]; >> >> + av_md5_init(ctx); >> >> + for (int j = 0; j < h; j++) { >> >> + av_md5_update(ctx, src, w); >> >> + src += stride; >> >> + } >> >> + av_md5_final(ctx, md5); >> >> + >> >> + if (memcmp(md5, expected, MD5_SIZE)) >> >> + return AVERROR_INVALIDDATA; >> >> + >> >> + return 0; >> >> +} >> >> + >> >> +static int verify_plane_crc(const uint8_t *src, const int w, const >> int h, const int stride, >> >> + const uint8_t *expected) >> >> +{ >> >> + uint32_t crc = 0x0F1D; // CRC-16-CCITT-AUG >> >> + const AVCRC *ctx = av_crc_get_table(AV_CRC_16_CCITT); >> >> + >> >> + for (int j = 0; j < h; j++) { >> >> + crc = av_crc(ctx, crc, src, w); >> >> + src += stride; >> >> + } >> >> + crc = av_bswap16(crc); >> >> + >> >> + if (memcmp(&crc, expected, sizeof(uint16_t))) >> > >> > This seems endian-dependent (as are the other checks). Why don't you >> > simply pass the proper CRC as uint16_t/uint32_t parameter? >> > >> >> + return AVERROR_INVALIDDATA; >> >> + >> >> + return 0; >> >> +} >> >> + >> >> +#define CAL_CHECKSUM(pixel) (checksum + ((pixel) ^ xor_mask)) >> >> +static int verify_plane_checksum(const uint8_t *src, const int w, >> const int h, const int stride, const int ps, >> >> + const uint8_t *expected) >> >> +{ >> >> + uint32_t checksum = 0; >> >> + >> >> + for (int y = 0; y < h; y++) { >> >> + for (int x = 0; x < w; x++) { >> >> + const int xor_mask = (x & 0xFF) ^ (y & 0xFF) ^ (x >> 8) ^ >> (y >> 8); >> >> + checksum = CAL_CHECKSUM(src[x << ps]); >> >> + if (ps) >> >> + checksum = CAL_CHECKSUM(src[(x << ps) + 1]); >> > >> > Why don't you use checksum += instead of using checksum in the >> > CAL_CHECKSUM macro in a hardcoded manner? >> > >> >> + } >> >> + src += stride; >> >> + } >> >> + >> >> + if (checksum != *(uint32_t*)expected) >> >> + return AVERROR_INVALIDDATA; >> >> + >> >> + return 0; >> >> +} >> >> + >> >> +static const uint8_t *get_plane_hash(const H274SEIPictureHash *h, >> const int plane) >> >> +{ >> >> + if (!h->hash_type) >> >> + return h->md5[plane]; >> >> + if (h->hash_type == 1) >> >> + return (uint8_t*)&h->crc[plane]; >> >> + return (uint8_t*)&h->checksum[plane]; >> >> +} >> > >> > This function seems completely useless; you should better pass the >> > proper expected value to the functions directly (by value for non-md5). >> > >> >> + >> >> +enum { >> >> + HASH_MD5SUM, >> >> + HASH_CRC, >> >> + HASH_CHECKSUM, >> >> + HASH_LAST = HASH_CHECKSUM, >> >> +}; >> >> + >> >> +struct H274HashContext { >> >> + int type; >> >> + struct AVMD5 *ctx; >> >> +}; >> >> + >> >> +int ff_h274_hash_freep(H274HashContext **ctx) >> >> +{ >> >> + if (ctx && *ctx) { >> > >> > Don't check for ctx. Passing a NULL to a freep-function is insane. >> > >> >> + H274HashContext *c = *ctx; >> >> + if (c->ctx) >> >> + av_free(c->ctx); >> >> + av_freep(ctx); >> >> + } >> >> + return 0; >> > >> > Why is this function returning anything? >> > >> >> +} >> >> + >> >> +int ff_h274_hash_init(H274HashContext **ctx, const int type) >> >> +{ >> >> + H274HashContext *c; >> >> + >> >> + if (type > HASH_LAST || !ctx) >> >> + return AVERROR(EINVAL); >> >> + >> >> + c = *ctx; >> >> + if (c) { >> >> + if (c->type != type) { >> >> + if (c->type == HASH_MD5SUM) >> >> + av_freep(&c->ctx); >> >> + c->type = type; >> >> + } >> >> + } else { >> >> + *ctx = c = av_mallocz(sizeof(H274HashContext)); >> > >> > Why is this separately allocated? >> > >> >> + if (!c) >> >> + return AVERROR(ENOMEM); >> >> + c->type = type; >> >> + } >> >> + >> >> + if (type == HASH_MD5SUM && !c->ctx) { >> >> + c->ctx = av_md5_alloc(); >> >> + if (!c->ctx) >> >> + return AVERROR(ENOMEM); >> >> + } >> >> + >> >> + return 0; >> >> +} >> >> + >> >> +int ff_h274_hash_verify(H274HashContext *c, const H274SEIPictureHash >> *hash, >> >> + const AVFrame *frame, const int coded_width, const int >> coded_height) >> >> +{ >> >> + const AVPixFmtDescriptor *desc; >> >> + int err = 0; >> >> + >> >> + if (!c || !hash || !frame) >> >> + return AVERROR(EINVAL); >> >> + >> >> + if (c->type != hash->hash_type) >> >> + return AVERROR(EINVAL); >> >> + >> >> + desc = av_pix_fmt_desc_get(frame->format); >> >> + if (!desc) >> >> + return AVERROR(EINVAL); >> >> + >> >> + for (int i = 0; i < desc->nb_components; i++) { >> >> + const int w = i ? (coded_width >> >> desc->log2_chroma_w) : coded_width; >> >> + const int h = i ? (coded_height >> >> desc->log2_chroma_h) : coded_height; >> >> + const int ps = desc->comp[i].step - 1; >> >> + const uint8_t *expected = get_plane_hash(hash, i); >> >> + const uint8_t *src = frame->data[i]; >> >> + const int stride = frame->linesize[i]; >> >> + >> >> + if (c->type == HASH_MD5SUM) >> >> + err = verify_plane_md5(c->ctx, src, w << ps, h, stride, >> expected); >> >> + else if (c->type == HASH_CRC) >> >> + err = verify_plane_crc(src, w << ps, h, stride, expected); >> >> + else if (c->type == HASH_CHECKSUM) >> >> + err = verify_plane_checksum(src, w, h, stride, ps, >> expected); >> >> + if (err < 0) >> >> + goto fail; >> >> + } >> >> + >> >> +fail: >> >> + return err; >> >> +} >> >> diff --git a/libavcodec/h274.h b/libavcodec/h274.h >> >> index e1803edaf3..e2bb1aec55 100644 >> >> --- a/libavcodec/h274.h >> >> +++ b/libavcodec/h274.h >> >> @@ -64,6 +64,8 @@ int ff_h274_apply_film_grain(AVFrame *out, const >> AVFrame *in, >> >> H274FilmGrainDatabase *db, >> >> const AVFilmGrainParams *params); >> >> >> >> +typedef struct H274HashContext H274HashContext; >> >> + >> >> typedef struct H274SEIPictureHash { >> >> int present; >> >> union { >> >> @@ -74,4 +76,9 @@ typedef struct H274SEIPictureHash { >> >> uint8_t hash_type; >> >> } H274SEIPictureHash; >> >> >> >> +int ff_h274_hash_init(H274HashContext **c, int type); >> >> +int ff_h274_hash_verify(H274HashContext *c, const H274SEIPictureHash >> *hash, >> >> + const AVFrame *frame, int coded_width, int coded_height); >> >> +int ff_h274_hash_freep(H274HashContext **c); >> >> + >> >> #endif /* AVCODEC_H274_H */ >> >> Hi Andreas, >> >> Thanks for your reviews! They help a lot. I've updated the v2 >> patchset based on your comments. Would you help to review the >> v2 patches? >> > Hi Andreas, > Thank you for the review. > > Hi Andreas and all, > I will push this next week unless there are any objections. > Applied. Thanks > > Thank you. > >> >> Thanks, >> Jianhua >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2025-05-31 1:53 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2025-04-01 17:15 [FFmpeg-devel] [PATCH v1 01/19] avcodec/cbs_sei_syntax_template: add sei message film_grain_characteristics toqsxw 2025-04-01 17:15 ` [FFmpeg-devel] [PATCH v1 02/19] avcodec/vvc: support decoding prefix and suffix nal units toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 03/19] avcodec/vvc/sei: add decode_film_grain_characteristics toqsxw 2025-04-25 22:11 ` Andreas Rheinhardt 2025-04-26 10:56 ` [FFmpeg-devel] 回复: " Wu Jianhua 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 04/19] avcodec/vvc/dec: export sei to the frame when the frame starts toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 05/19] avcodec/vvc/dec: support applying film grain by the decoder toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 06/19] avcodec/vvc/dec: support removing film grain params from side data toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 07/19] avcodec/h274: add H274SEIPictureHash struct toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 08/19] avcodec/vvc/sei: add decode_decoded_picture_hash toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 09/19] avcodec/h274: add ff_h274_hash functions toqsxw 2025-04-25 22:01 ` Andreas Rheinhardt 2025-05-08 18:36 ` [FFmpeg-devel] 回复: " Wu Jianhua 2025-05-24 14:02 ` Nuo Mi 2025-05-31 1:53 ` Nuo Mi [this message] 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 10/19] avcodec/vvcdec: verify picture hash toqsxw 2025-04-25 22:02 ` Andreas Rheinhardt 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 11/19] avcodec/cbs_sei_syntax_template: add sei message sei_display_orientation toqsxw 2025-04-25 22:05 ` Andreas Rheinhardt 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 12/19] avcodec/vvc/sei: add decode_display_orientation toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 13/19] avcodec/vvc/sei: add decode_content_light_level_info toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 14/19] avcodec/cbs_sei_syntax_template: add sei message frame_field_information toqsxw 2025-04-25 22:08 ` Andreas Rheinhardt 2025-04-25 22:12 ` James Almer 2025-04-25 22:55 ` Andreas Rheinhardt 2025-04-25 22:58 ` James Almer 2025-04-25 23:02 ` Andreas Rheinhardt 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 15/19] avcodec/h274: add H274SEIFrameFieldInfo struct toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 16/19] avcodec/vvc/sei: add decode_frame_field_info toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 17/19] avcodec/vvc: support fields toqsxw 2025-04-01 17:35 ` Andreas Rheinhardt 2025-04-01 18:11 ` [FFmpeg-devel] 回复: " Wu Jianhua 2025-04-23 13:54 ` Nuo Mi 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 18/19] avcodec/vvc/sei: add decode_ambient_viewing_environment toqsxw 2025-04-01 17:16 ` [FFmpeg-devel] [PATCH v1 19/19] avcodec/vvc/sei: add decode_mastering_display_colour_volume toqsxw 2025-04-25 21:37 ` [FFmpeg-devel] [PATCH v1 01/19] avcodec/cbs_sei_syntax_template: add sei message film_grain_characteristics Andreas Rheinhardt 2025-04-26 11:37 ` [FFmpeg-devel] 回复: " Wu Jianhua
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAFXK13emh1RGQDh9QfRa4hEypbqUaNs8b=WXD=Q0XsOopJhR-Q@mail.gmail.com' \ --to=nuomi2021@gmail.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git