[FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

Film grain support adds a huge amount of overhead to the H264Context
structure for a feature that is rarely used. On low end devices or
pages that have lots of media this bloats memory usage rapidly.

This introduces a --disable-h264-film-grain option which makes
these fields optional and reduces the H264Context size from
851808 bytes to 53444 bytes.

Bug: https://crbug.com/359358875
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>

Note: I'm not sure this is the right way to go about making this optional,
please
let me know if there's a better way.

- dale

[-- Attachment #2: optional_film_grain_v1.patch --]
[-- Type: application/octet-stream, Size: 8258 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[James Almer]

On 8/13/2024 4:31 PM, Dale Curtis wrote:
> Film grain support adds a huge amount of overhead to the H264Context
> structure for a feature that is rarely used. On low end devices or
> pages that have lots of media this bloats memory usage rapidly.
> 
> This introduces a --disable-h264-film-grain option which makes
> these fields optional and reduces the H264Context size from
> 851808 bytes to 53444 bytes.
> 
> Bug: https://crbug.com/359358875
> Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
> 
> Note: I'm not sure this is the right way to go about making this optional,
> please
> let me know if there's a better way.
> 
> - dale

The proper name for the option and define should be H274, or simply 
film_grain if you're also including AV1FG in it.
I'm not against a change like this, but it needs to be thorough like we 
did with iamfenc and iamfdec, and there's more code handling film grain 
in other modules.

Not sure what Niklas thinks.

_______________________________________________
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".

Re: [FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[Dale Curtis]

Thanks, disable-h274-film-grain and applying it to hevc too sgtm. I'll wait
to see what Niklas says before updating though.

- dale

On Tue, Aug 13, 2024 at 12:47 PM James Almer <jamrial@gmail.com> wrote:

> On 8/13/2024 4:31 PM, Dale Curtis wrote:
> > Film grain support adds a huge amount of overhead to the H264Context
> > structure for a feature that is rarely used. On low end devices or
> > pages that have lots of media this bloats memory usage rapidly.
> >
> > This introduces a --disable-h264-film-grain option which makes
> > these fields optional and reduces the H264Context size from
> > 851808 bytes to 53444 bytes.
> >
> > Bug: https://crbug.com/359358875
> > Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
> >
> > Note: I'm not sure this is the right way to go about making this
> optional,
> > please
> > let me know if there's a better way.
> >
> > - dale
>
> The proper name for the option and define should be H274, or simply
> film_grain if you're also including AV1FG in it.
> I'm not against a change like this, but it needs to be thorough like we
> did with iamfenc and iamfdec, and there's more code handling film grain
> in other modules.
>
> Not sure what Niklas thinks.
>
> _______________________________________________
> 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".

Re: [FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[Hendrik Leppkes]

On Tue, Aug 13, 2024 at 9:32 PM Dale Curtis <dalecurtis@chromium.org> wrote:
>
> Film grain support adds a huge amount of overhead to the H264Context
> structure for a feature that is rarely used. On low end devices or
> pages that have lots of media this bloats memory usage rapidly.
>
> This introduces a --disable-h264-film-grain option which makes
> these fields optional and reduces the H264Context size from
> 851808 bytes to 53444 bytes.
>
> Bug: https://crbug.com/359358875
> Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
>

Disabling random codec features seems like an anti-feature to me, in
the future it'll make every feature be questioned and compile-time
conditional, and make everything terrible.
If the context size is the major concern, maybe large structures
should be allocated when in use, rather than always?

- Hendrik
_______________________________________________
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".

Re: [FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

On Tue, Aug 13, 2024 at 1:11 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> Disabling random codec features seems like an anti-feature to me, in
> the future it'll make every feature be questioned and compile-time
> conditional, and make everything terrible.
> If the context size is the major concern, maybe large structures
> should be allocated when in use, rather than always?
>

I agree with that, so here's a version which allocates dynamically instead.
It passes FATE, but I didn't try with valgrind/msan in case I missed a
cleanup path.

- dale

[-- Attachment #2: dynamic_film_grain_v0.patch --]
[-- Type: application/octet-stream, Size: 11878 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] PATCH] Make H.274 film grain support optional for H.264. Saves ~779kb.

[Christophe Gisquet]

Hi,

Le mar. 13 août 2024 à 23:39, Dale Curtis <dalecurtis@chromium.org> a écrit :
>
> On Tue, Aug 13, 2024 at 1:11 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> > Disabling random codec features seems like an anti-feature to me, in
> > the future it'll make every feature be questioned and compile-time
> > conditional, and make everything terrible.
> > If the context size is the major concern, maybe large structures
> > should be allocated when in use, rather than always?
> >
>
> I agree with that, so here's a version which allocates dynamically instead.
> It passes FATE, but I didn't try with valgrind/msan in case I missed a
> cleanup path.

(not an author)
Probably there is remaining work pending on external dependencies, but
H274FilmGrainDatabase sounds a bit adhoc. The 8KB of slice_tmp for one
could just be a local variable in init_slice(). The 676KB (!) of the
noise database are a bit more puzzling and could be the one that needs
allocating only when the noise is generated. It may simplify a bit
some of the conditions in this patch.

-- 
Christophe
_______________________________________________
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".