[SkVideoEncoder] Allow clients to control the color type
Change-Id: If57514c9e7e8d0a417eb9388873bbb348fc49076
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/247384
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/experimental/ffmpeg/SkVideoEncoder.cpp b/experimental/ffmpeg/SkVideoEncoder.cpp
index 96daeb6..358f5a5 100644
--- a/experimental/ffmpeg/SkVideoEncoder.cpp
+++ b/experimental/ffmpeg/SkVideoEncoder.cpp
@@ -231,10 +231,9 @@
return false;
}
- // need opaque and bgra to efficiently use libyuv / convert-to-yuv-420
SkAlphaType alphaType = kOpaque_SkAlphaType;
sk_sp<SkColorSpace> cs = nullptr; // should we use this?
- fInfo = SkImageInfo::Make(dim, kRGBA_8888_SkColorType, alphaType, cs);
+ fInfo = SkImageInfo::MakeN32(dim.width(), dim.height(), alphaType, cs);
if (!this->init(fps)) {
return false;
}
@@ -242,14 +241,15 @@
fCurrentPTS = 0;
fDeltaPTS = 1;
- SkASSERT(sws_isSupportedInput(AV_PIX_FMT_RGBA) > 0);
+ const auto fmt = kN32_SkColorType == kRGBA_8888_SkColorType ? AV_PIX_FMT_RGBA : AV_PIX_FMT_BGRA;
+ SkASSERT(sws_isSupportedInput(fmt) > 0);
SkASSERT(sws_isSupportedOutput(AV_PIX_FMT_YUV420P) > 0);
// sws_getCachedContext takes in either null or a previous ctx. It returns either a new ctx,
// or the same as the input if it is compatible with the inputs. Thus we never have to
// explicitly release our ctx until the destructor, since sws_getCachedContext takes care
// of freeing the old as needed if/when it returns a new one.
fSWScaleCtx = sws_getCachedContext(fSWScaleCtx,
- dim.width(), dim.height(), AV_PIX_FMT_RGBA,
+ dim.width(), dim.height(), fmt,
dim.width(), dim.height(), AV_PIX_FMT_YUV420P,
SWS_FAST_BILINEAR, nullptr, nullptr, nullptr);
return fSWScaleCtx != nullptr;
@@ -259,6 +259,9 @@
if (!is_valid(pm.dimensions())) {
return false;
}
+ if (pm.info().colorType() != fInfo.colorType()) {
+ return false;
+ }
/* make sure the frame data is writable */
if (check_err(av_frame_make_writable(fFrame))) {
return false;
diff --git a/experimental/ffmpeg/SkVideoEncoder.h b/experimental/ffmpeg/SkVideoEncoder.h
index 6a35061..62450e4 100644
--- a/experimental/ffmpeg/SkVideoEncoder.h
+++ b/experimental/ffmpeg/SkVideoEncoder.h
@@ -28,21 +28,15 @@
~SkVideoEncoder();
/**
- * Begina a new recording. Balance this (after adding all of your frames) with a call
+ * Begins a new recording. Balance this (after adding all of your frames) with a call
* to endRecording().
*/
bool beginRecording(SkISize, int fps);
/**
- * Returns the preferred ImageInfo for this recording. Only valid if beginRecording() has
- * been called, and endRecording has not been called (yet).
- */
- SkImageInfo preferredInfo() const { return fInfo; }
-
- /**
* If you have your own pixmap, call addFrame(). Note this may fail if it uses an unsupported
- * ColorType or AlphaType, or the dimensions don't match those set in beginRecording.
- * For best results, use the SkImageInfo returned by preferredInfo().
+ * ColorType (requires kN32_SkColorType) or AlphaType, or the dimensions don't match those set
+ * in beginRecording.
*/
bool addFrame(const SkPixmap&);
diff --git a/modules/skottie/src/SkottieTool.cpp b/modules/skottie/src/SkottieTool.cpp
index 4f516a7..483bcfe 100644
--- a/modules/skottie/src/SkottieTool.cpp
+++ b/modules/skottie/src/SkottieTool.cpp
@@ -173,7 +173,7 @@
const sk_sp<SkSurface> fSurface;
};
-static std::vector<SkBitmap> gMP4Frames;
+static std::vector<sk_sp<SkImage>> gMP4Frames;
struct MP4Sink final : public Sink {
explicit MP4Sink(const SkMatrix& scale_matrix)
@@ -188,9 +188,8 @@
}
bool endFrame(size_t i) override {
- // SkVideoEncoder wants RGBA 8888 frames. (N32 may be RGBA 8888 or BGRA 8888.)
- gMP4Frames[i].allocPixels(fSurface->imageInfo().makeColorType(kRGBA_8888_SkColorType));
- return fSurface->readPixels(gMP4Frames[i].pixmap(), 0,0);
+ gMP4Frames[i] = fSurface->makeImageSnapshot();
+ return SkToBool(gMP4Frames[i]);
}
const sk_sp<SkSurface> fSurface;
@@ -344,9 +343,14 @@
#if defined(HAVE_VIDEO_ENCODER)
if (FLAGS_format.contains("mp4")) {
SkVideoEncoder enc;
- enc.beginRecording({FLAGS_width, FLAGS_height}, FLAGS_fps);
- for (const SkBitmap& frame : gMP4Frames) {
- enc.addFrame(frame.pixmap());
+ if (!enc.beginRecording({FLAGS_width, FLAGS_height}, FLAGS_fps)) {
+ SkDEBUGF("Invalid video stream configuration.\n");
+ return -1;
+ }
+ for (const auto& frame : gMP4Frames) {
+ SkPixmap pm;
+ SkAssertResult(frame->peekPixels(&pm));
+ enc.addFrame(pm);
}
sk_sp<SkData> mp4 = enc.endRecording();
diff --git a/tools/skottie2movie.cpp b/tools/skottie2movie.cpp
index 48d5dee..e12ad3c 100644
--- a/tools/skottie2movie.cpp
+++ b/tools/skottie2movie.cpp
@@ -100,11 +100,14 @@
sk_sp<SkSurface> surf;
sk_sp<SkData> data;
+ const auto info = SkImageInfo::MakeN32Premul(dim);
do {
double loop_start = SkTime::GetSecs();
- encoder.beginRecording(dim, fps);
- auto info = encoder.preferredInfo();
+ if (!encoder.beginRecording(dim, fps)) {
+ SkDEBUGF("Invalid video stream configuration.\n");
+ return -1;
+ }
// lazily allocate the surfaces
if (!surf) {
