[vlmc-devel] [PATCH 08/10] Workflow::Frame: Don't remember VideoTrack-specific data
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Wed May 18 21:44:40 CEST 2016
On 05/04/2016 04:18 PM, Yikai Lu wrote:
> ---
> src/EffectsEngine/EffectUser.cpp | 2 +-
> src/Workflow/Types.cpp | 69 ++++------------------------------------
> src/Workflow/Types.h | 23 --------------
> 3 files changed, 7 insertions(+), 87 deletions(-)
>
> diff --git a/src/EffectsEngine/EffectUser.cpp b/src/EffectsEngine/EffectUser.cpp
> index e9ebb75..8816481 100644
> --- a/src/EffectsEngine/EffectUser.cpp
> +++ b/src/EffectsEngine/EffectUser.cpp
> @@ -99,7 +99,7 @@ EffectUser::applyFilters( const Workflow::Frame* frame, qint64 currentFrame )
> else
> buff = &buff2;
> if ( *buff == nullptr )
> - *buff = new quint32[frame->nbPixels()];
> + *buff = new quint32[ m_width * m_height * Workflow::Depth ];
> EffectInstance *effect = (*it)->effectInstance();
> effect->process( input, *buff );
> input = *buff;
> diff --git a/src/Workflow/Types.cpp b/src/Workflow/Types.cpp
> index 74b5ff6..94efd18 100644
> --- a/src/Workflow/Types.cpp
> +++ b/src/Workflow/Types.cpp
> @@ -29,11 +29,8 @@ using namespace Workflow;
> Frame::Frame() :
> OutputBuffer( VideoTrack ),
> ptsDiff( 0 ),
> - m_width( 0 ),
> - m_height( 0 ),
> m_buffer( 0 ),
> m_size( 0 ),
> - m_nbPixels( 0 ),
> m_pts( 0 )
> {
> }
> @@ -41,38 +38,19 @@ Frame::Frame() :
> Frame::Frame( quint32 width, quint32 height ) :
> OutputBuffer( VideoTrack ),
> ptsDiff( 0 ),
> - m_width( width ),
> - m_height( height ),
> m_pts( 0 )
> {
> - m_nbPixels = width * height;
> - m_size = m_nbPixels * Depth;
> - m_buffer = new quint32[m_nbPixels];
> + m_size = width * height * Depth;
> + m_buffer = new quint32[width * height];
> }
>
> Frame::Frame( size_t forcedSize ) :
> OutputBuffer( VideoTrack ),
> ptsDiff( 0 ),
> - m_width( 0 ),
> - m_height( 0 ),
> - m_size( forcedSize ),
> - m_nbPixels( 0 ),
> m_pts( 0 )
> {
> - m_buffer = new quint32[ ( forcedSize % 4 ) ? forcedSize / 4 + 1 : forcedSize / 4 ];
> -}
> -
> -Frame::Frame(quint32 width, quint32 height, size_t forcedSize) :
> - OutputBuffer( VideoTrack ),
> - ptsDiff( 0 ),
> - m_width( width ),
> - m_height( height ),
> - m_pts( 0 )
> -{
> - m_nbPixels = width * height;
> m_size = forcedSize;
> - Q_ASSERT(forcedSize % 4 == 0);
> - m_buffer = new quint32[forcedSize / 4];
> + m_buffer = new quint32[ ( forcedSize % 4 ) ? forcedSize / 4 + 1 : forcedSize / 4 ];
> }
>
> Frame::~Frame()
> @@ -91,29 +69,11 @@ const quint32 *Frame::buffer() const
> return m_buffer;
> }
>
> -quint32
> -Frame::width() const
> -{
> - return m_width;
> -}
> -
> -quint32
> -Frame::height() const
> -{
> - return m_height;
> -}
> -
> size_t Frame::size() const
> {
> return m_size;
> }
>
> -quint32
> -Frame::nbPixels() const
> -{
> - return m_nbPixels;
> -}
> -
> qint64
> Frame::pts() const
> {
> @@ -126,11 +86,6 @@ Frame::setPts( qint64 pts )
> m_pts = pts;
> }
>
> -size_t Frame::Size(quint32 width, quint32 height)
> -{
> - return width * height * Depth;
> -}
> -
> void
> Frame::setBuffer( quint32 *buff )
> {
> @@ -139,23 +94,11 @@ Frame::setBuffer( quint32 *buff )
> }
>
> void
> -Frame::resize( quint32 width, quint32 height )
> -{
> - if ( width != m_width || height != m_height )
> - {
> - delete[] m_buffer;
> - m_width = width;
> - m_height = height;
> - m_nbPixels = width * height;
> - m_size = m_nbPixels * Depth;
> - m_buffer = new quint32[m_nbPixels];
> - }
> -}
> -
> -void Frame::resize(size_t size)
> +Frame::resize( size_t size )
> {
> if ( size == m_size )
> return ;
> delete[] m_buffer;
> - m_buffer = new quint32[size / sizeof(quint32)];
> + m_size = size;
> + m_buffer = new quint32[ ( size % 4 ) ? size / 4 + 1 : size / 4 ];
If we're going to store buffers without them having to be a frame, I
think we should store a quint8*, and comply to the size being requested.
Plus, this check makes no sense in the context of an audio buffer.
> }
> diff --git a/src/Workflow/Types.h b/src/Workflow/Types.h
> index 3baeab3..1b289a0 100644
> --- a/src/Workflow/Types.h
> +++ b/src/Workflow/Types.h
> @@ -53,20 +53,11 @@ namespace Workflow
> explicit Frame();
> Frame( quint32 width, quint32 height );
> Frame( size_t forcedSize );
> - Frame( quint32 width, quint32 height, size_t forcedSize );
> ~Frame();
> - quint32 width() const;
> - quint32 height() const;
> quint32 *buffer();
> const quint32 *buffer() const;
> void setBuffer( quint32 *buff );
> /**
> - * \brief Resize the buffer.
> - *
> - * \warning This will not resize what's in the frame but drop it instead!
> - */
> - void resize( quint32 width, quint32 height );
> - /**
> * \brief Resize the buffer
> *
> * This will allocate a new buffer and drop the old one.
> @@ -80,12 +71,6 @@ namespace Workflow
> */
> size_t size() const;
> /**
> - * \returns The frame size in pixels
> - *
> - * This is equal to width * height
> - */
> - quint32 nbPixels() const;
> - /**
> * \brief Get pts.
> *
> */
> @@ -103,15 +88,7 @@ namespace Workflow
> //FIXME
> qint64 ptsDiff;
>
> - public:
> - /**
> - * @brief Size Computes the size, in bytes, a frame with given dimension would use.
> - */
> - static size_t Size( quint32 width, quint32 height );
> -
> private:
> - quint32 m_width;
> - quint32 m_height;
> // frei0r uses 32bits only pixels, and expects its buffers as uint32
> quint32 *m_buffer;
> size_t m_size;
>
While I agree this is a step in a good direction to share a single class
to hold the result of all ClipWorkflow::getOutput, I still think it
would be handy to have some basic informations about what's being
stored, and I'm not sure we should remove methods such as width/height.
I'm a bit more enclined to store everything inside as a union, and keep
track of the type of the buffer.
--
Hugo Beauzée-Luyssen
www.beauzee.fr
More information about the Vlmc-devel
mailing list