Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] video/image_writer: add bgra screenshot support #12149

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

po5
Copy link
Contributor

@po5 po5 commented Aug 13, 2023

Mostly for use in scripts that will re-use the screenshots in overlay-add like thumbfast.
The advantage of this over encode mode (which TheAMM's thumbnailer and thumbfast use) is that we can accurately match the user's tonemapping, and screenshots allow dynamic side data in the filename to solve po5/thumbfast#97 (encode mode output filename can't be updated at runtime)

Not ready for merge as it sometimes initially outputs all 0's on HDR files with gpu-next. Making a 2nd screenshot of the same frame correctly renders it.
Does anyone know why this is happening?

@@ -706,6 +707,9 @@ bool write_image(struct mp_image *image, const struct image_writer_opts *opts,
int alpha = image->fmt.flags & MP_IMGFLAG_ALPHA;
destfmt = alpha ? pixfmt2imgfmt(AV_PIX_FMT_YUVA420P) : IMGFMT_420P;
}
if (opts->format == AV_CODEC_ID_RAWVIDEO) {
destfmt = IMGFMT_BGRA;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're better off writing a new function that dumps the mp_image contents than trying to reuse write_lavc

@@ -557,6 +557,8 @@ Available video output drivers are:
PNG files.
webp
WebP files.
bgra
BGRA files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like

Suggested change
BGRA files.
Raw BGRA (32bpp) pixel data without header or padding.

would be more descriptive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants