[vlmc-devel] [PATCH 08/10] Workflow::Frame: Don't remember VideoTrack-specific data
yikei lu
luyikei.qmltu at gmail.com
Thu May 19 01:09:26 CEST 2016
> 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.
Well, I think renderers should know such information. So there is no
need to remember it expect for size. But I don't deny that it's handy
:)
2016-05-19 4:44 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
> 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
> _______________________________________________
> Vlmc-devel mailing list
> Vlmc-devel at videolan.org
> https://mailman.videolan.org/listinfo/vlmc-devel
More information about the Vlmc-devel
mailing list