Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
16693005:
Add ExynosVideoEncodeAccelerator (Closed)
|
Created:
7 years, 6 months ago by sheu Modified:
7 years, 1 month ago CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, apatrick_chromium, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@screencast_cl_5 Visibility:
Public. |
DescriptionAdd ExynosVideoEncodeAccelerator
* Add ExynosVideoEncodeAccelerator, for use in
hardware-accelerating video encoding on Exynos platforms.
* Add a new profile RAWPROFILE_I420 to media::VideoCodecProfile to indicate an
packed I420 bitstream.
BUG=chromium:221441
TEST=build, run on CrOS snow
Change-Id: I8bdf867f0d1dfbe3e10ee9279b327a05ce1d2ff0
Patch Set 1 : 7e087a11 #Patch Set 2 : 11c25090 Rebase. #Patch Set 3 : b4091995git log --decorate #Patch Set 4 : b4091995 MFC hardware requirements. #Patch Set 5 : a1d2f2c4 Bitrate settings. #
Total comments: 82
Patch Set 6 : b673f1a3 WIP - for posciak@ #
Messages
Total messages: 5 (0 generated)
The encoder. fischman@: PTAL
First pass. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... File content/common/gpu/media/exynos_video_encode_accelerator.cc (right): https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:278: // 1. Format (VIDIOC_S_FMT) set first on OUTPUT and CPATURE queues. s/CPATURE/CAPTURE/ https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:477: if (frame->coded_size() != input_visible_size_) { This is confusing... why not visible_rect() (needs at least doco)? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:510: // DestroyTask() should run regardless of encoder_state_. Perhaps a DCHECK(!decoder_thread_.IsRunning() || decoder_thread_.message_loop() == base::MessageLoop::current()) ? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:673: dqbuf.length = 3; 3 planes for GSC CAPTURE, but always 2 into MFC OUTPUT? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:825: eglClientWaitSyncKHR(egl_display_, input_record.egl_sync, 0, egl calls from encoder_thread_? Also, needs MCC? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1215: control.value = 1; Is this because our textures are flipped？ https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1386: buffer.length = arraysize(output_record.address); s/arraysize(output_record.address)/arraysize(planes)/ https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1414: format.fmt.pix_mp.width = converted_visible_size_.width(); All those sizes confuse me to no end, unfortunately. But shouldn't width/height be coded size? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1473: parms.parm.output.timeperframe.numerator = 1000; I'm surprised this is required. Why? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1565: IOCTL_OR_ERROR_RETURN(mfc_fd_, VIDIOC_S_EXT_CTRLS, &control); Is not changing the bitrate a noncritical error that doesn't require NotifyError? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... File content/common/gpu/media/exynos_video_encode_accelerator.h (right): https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:62: const gfx::Size& input_resolution, Do you know if the codec supports in-flight resolution changes? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:149: // Decoding tasks, to be run on encode_thread_. s/encode_thread_/encoder_thread_/ https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:179: // Return completed buffers to the client. Doco which buffers (bitstream buffers)? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:263: gfx::Size converted_visible_size_; Perhaps I'm misunderstanding the intentions here, but even if GSC and MFC have different size requirements and if it also depends on format, wouldn't it be better performance-wise to scale it once to a common size for all initially, on first conversion/scale operation and use one size for converted_visible and output_visible sizes? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:279: // Flag if we are encoding the output (and using MFC). In what scenario we'd not want to encode? Is this for testing/debug only? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:304: std::list<int> gsc_output_mfc_input_queue_; s/waiting for GSC/waiting for MFC/ ?
https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... File content/common/gpu/gl_surface_encoder.cc (right): https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/gl_surface_encoder.cc:1: // Copyright (c) 2013 The Chromium Authors. All rights reserved. Can you include a teaser of perf #'s for video encode in the CL description? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/gl_surface_encoder.cc:145: gfx::GLSurfaceEGL::GetHardwareDisplay(), Any reason not to do this in the EVDA ctor itself? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... File content/common/gpu/media/exynos_video_encode_accelerator.cc (right): https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:7: #include <dlfcn.h> copy/pasta? (please make a pass on #includes and drop unnecessary ones) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:50: namespace { anon namespace buys you nothing and costs you: - 4 lines of vertical space - harder debuggability compared to "static" prefix. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:74: texture_id(0) {} address & length are uninitialized. please make a pass to ensure all fields are init'd by all ctors. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:79: address[0] = address[1] = address[2] = NULL; I am surprised to find out that the style guide doesn't forbid multiple assignment (and that in fact it has nothing about "only one statement per line"). You could use memset or even array initialization (={0}). Your choice. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:120: device_poll_thread_("ExynosDevicePollThread"), This is the same name used by EVDA. IWBN to have different names. (well, it would be even nicer not to have these threads in the first place, but that's the TODO in the header). https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:125: DCHECK(client_); DCHECK_EQ(base::MessageLoopProxy::current(), ChildThread::current()->message_loop()->message_loop_proxy()); here and in the dtor to ensure new/delete on the child thread? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:208: gsc_fd_ = HANDLE_EINTR(open(kExynosGscDevice, worth pulling out open+capscheck into a helper function that you can call twice (once for gsc, once for mfc)? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:262: << input_visible_size_.width() << "x" input_visible_size_.ToString() and ditto for the rest. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:322: gsc_free_input_buffers_.push_back(index); I was going to observe that ReturnFreeGscInputBuffer(index); is not much of a typings savings over gsc_free_input_buffers_.push_back(index); and also has a non-zero abstraction cost, so looked to see how many times it's used (to gauge usefulness). It's called a grand total of never times. Remove? ETA: but, it is racy that g_f_i_b_ is read & written on the encoder thread! So really the helper _was_ a good idea (b/c it could DCHECK the thread-correctness) and you need a similar one for the read/pop, but need to start using them. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:336: frame_id)); Silently drop the frame? At least worthy of a comment here, but more likely we'll want to know about it in some way, no? Ideally we'd want this to be reflected in webrtc stats (in JS). Probably for now it's enough to UMA_HISTOGRAM here, with a TODO to add a status param to VEA::C::NEOVF() reflecting success/drop. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:340: if (!make_context_current_.Run()) { Comment this is needed for the eglsync call? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:354: if (frame->format() == media::VideoFrame::NATIVE_EGLSURFACE) { WDYT of putting at the top of this method: if ((frame->format() == media::VideoFrame::NATIVE_EGLSURFACE) != do_encode_from_backbuffer_) { NOTIFY_ERROR(INVALID_PARAMS); return; } And then instead of comparing to NATIVE_EGLSURFACE branch on do_encode_from_backbuffer_ here and below. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:362: input_record.egl_sync = Why does this happen after the glCopyTexSubImage2D? (better, add a comment explaining what this is sync-pointing relative to?) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:392: new base::SharedMemory(buffer.handle(), false)).Pass(), wouldn't it be simpler to pass the handle to the BBR ctor?? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:461: &media::VideoEncodeAccelerator::Client::NotifyEndOfVideoFrame, In the case of encoding video frames backed by textures, wouldn't you want to wait till the CSC was done before doing this? ETA: but actually you do do this after GSC input is dequeued. So this is just spurious? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:487: NOTIMPLEMENTED(); TODOify the bits that aren't (namely, copying |frame| into |gsc_input_index|). https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:523: DCHECK_NE(encoder_state_, kInitialized); Could be more robust to future change by dropping these two lines and then DCHECK_EQ(encoder_state_, kEncoding); below the test for kError (l.530). https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:542: // GSC has to wait until both input and output buffers are queued. This logic makes me think it's never correct to have more than one ServiceDeviceTask or DevicePollTask posted task concurrently (i.e. total posted tasks for both functions of 0 or 1). IWBN if there was a way to assert that. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:555: // have early-outed already. either... is missing "or" clause. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:610: if (!EnqueueGscOutputRecord()) Why not loop until gsc_free_output_buffers_.empty() around this? That's what a reader who didn't see l.625 would ask. Pointer to there from here in comment please. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:644: if (ioctl(gsc_fd_, VIDIOC_DQBUF, &dqbuf) != 0) { HANDLE_EINTR? (here and elsewhere) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:666: while (gsc_output_buffer_queued_count_ > 0) { Note this only goes around at most once because of the bug described above? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:944: !encoder_bitstream_buffers_.empty()) { indent to opening parens (what happened to [git cl format]?) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:956: offset += output_record.bytes_used; s/+=/=/ ? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1022: return false; NOTIFY_ERROR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1026: return false; NOTIFY_ERROR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1098: const uint64 buf = 1; Why uint64 and thus more than a byte and thus the possibility that write() writes less than all 8 bytes in one go? I think this should be a uint8, and the read/write calls below should be testing for !=1 for the error paths. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1156: // Poll it! Add a trace event to make obvious to about:tracing what time is spent sitting in this poll? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1191: encoder_thread_.message_loop() != base::MessageLoop::current()) { indent https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1273: EGLint attrs[13]; Why not EGLint attrs[] = { EGL_WIDTH, i_a_s_.width(), ..., EGL_NONE }; ? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1394: return false; NOTIFY_ERROR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1440: output_buffer_byte_size_ = kMfcOutputBufferSize; Move to l.1335 to mirror the else clause up there? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1443: video_profile_ <= media::H264PROFILE_MAX) { Elsewhere you only accept Main for h264, so this is misleading. (it's also the only use of video_profile_ except for the function that sets the member, so I think you can drop the member and hardcode the pixelformat until more codecs are added to the mfc path. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1456: struct v4l2_ext_controls control; the type+name combos of the previous two lines are pure gold. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1460: ctrls[0].value = 0; does this mean no B frames? (why do that?) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1466: control.count = 3; I guess doco all these encoding params? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1474: parms.parm.output.timeperframe.denominator = 25; Is this hard-coding 40fps framerate? Frame duration be derived from capture rate? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1544: return false; NOTIFY_ERROR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1565: IOCTL_OR_ERROR_RETURN(mfc_fd_, VIDIOC_S_EXT_CTRLS, &control); v4l2 is thread safe - it's legit to ioctl mfc_fd_ from the child thread while the encoder thread is using it? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1588: if (ioctl(gsc_fd_, VIDIOC_REQBUFS, &reqbufs) != 0) HANDLE_EINTR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1614: if (ioctl(gsc_fd_, VIDIOC_REQBUFS, &reqbufs) != 0) HANDLE_EINTR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1641: if (ioctl(mfc_fd_, VIDIOC_REQBUFS, &reqbufs) != 0) HANDLE_EINTR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.cc:1665: if (ioctl(mfc_fd_, VIDIOC_REQBUFS, &reqbufs) != 0) HANDLE_EINTR? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... File content/common/gpu/media/exynos_video_encode_accelerator.h (right): https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:5: #ifndef CONTENT_COMMON_GPU_MEDIA_EXYNOS_VIDEO_ENCODE_ACCELERATOR_H_ TODO+crbug to add a vea_test. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:43: // on the Exynos platform. Can this be V4L2VEA instead of ExynosVEA? (what exynos-specifics make it unlikely to work on other v4l2 impls?) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:43: // on the Exynos platform. sanity check: no dlopen/dlsym's necessary for encode & scrape functionality? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:45: // The threading model of this class is the same as the Much like EVDA IWBN to have the poll thread replaced with a TYPE_IO encoder thread (https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:79: // Record for GSC input buffers. :'( for GSC staying in our lives. I take it there is no parallel for coming here? (i.e. have the GPU do the CSC instead of having gscaler do it) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:123: // Use maximum theoretical size to avoid hanging the hardware. TODO/crbug pointer https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:123: // Use maximum theoretical size to avoid hanging the hardware. What theory makes 2MB the "max theoretical size"? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:130: kInitialized, // Initialize() returned true; ready to start decoding. "decoding" https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:135: // File descriptors we need to poll. Reword to avoid implying that the actual FDs are 0 & 1. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:136: enum PollFds { can live in the .cc file https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:149: // Decoding tasks, to be run on encode_thread_. lol Decoding...encode... https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:175: bool EnqueueGscInputRecord(); here and below, doco that a false return means fatal error? (and handle it as such? I think you have a bunch of silent "return"s that might be burying errors) WARN_UNUSED_RESULT might be useful, too. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:185: // Set/clear the device poll interrupt (using device_ooll_interrupt_fd_). typo: ooll https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:216: bool SetMfcFormats(); Not really "creat[ing] the buffers we need". https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:257: // Encoder state machine state. There's no explicit "machine" so drop "machine state"? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:260: gfx::Size input_visible_size_; "visible" is usually a Rect, not a Size, and it can vary frame by frame. You sure it makes sense to track in members like this? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:273: // Buffers ready to be encoded. Here and elsewhere, doco what the int is an index into. https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:292: std::vector<GscInputRecord> gsc_input_buffer_map_; s/_map_/s_/ and similarly update comment since this isn't a map? (ditto for {gsc,mfc}_{output,input}_buffer_map_) https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:303: // Completed GSC outputs, waiting for GSC. lolwat? https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:345: std::vector<int> gsc_free_input_buffers_; This is both read & written on both the child thread & the encoder thread! https://chromiumcodereview.appspot.com/16693005/diff/22001/content/common/gpu... content/common/gpu/media/exynos_video_encode_accelerator.h:350: // The codec we'll be decoding for. encoding
sheu@ time to close this CL?
Old CL is old. Committed as https://chromiumcodereview.appspot.com/20962003/. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
