[vlc-devel] [PATCH 20/20] mkv: Reimplemented MkvTree and moved it to `Util.hpp`

Denis Charmet typx at dinauz.org
Mon Mar 7 11:49:34 CET 2016


Honestly I don't mind/care so nevermind :)

I was just surprised since all the previous patches are more about "not 
manually managing memory and using C++ stuff" seeing this pure old C 
looked kinda out of place so I had to ask.

On 2016-03-07 11:42, Filip Roséen wrote:
> The dynamic case is very rare (I have never seen an instance where it
> is actually hit), as such I used strcat instead of memcpy.
> 
> The new implementation is not meant to be a super performance
> increase (even though it aids a little since there is a lot of
> diagnostics going on when initially parsing the mkv), but mostly a
> rewrite of the old implementation with an added bonus (not doing
> dynamic initialization in 99.999..% of the cases).
> 
> One could use std::string for the purpose, but the C++03 grammar in
> terms of std::basic_string<...> is not really my favorite part of the
> Standard.
> 
>> In my opinion, adding branching just to potentially avoid dynamic 
>> allocation is debatable. I didn’t run a benchmark but it seems 
>> overkill in any case. If you are really concerned with the execution 
>> speed of a non critical function, I would argue that memcpy will be 
>> probably be faster than strcat :)
> 
> I agree with ou that branching is evil, and if you want to clean up
> the implementation you are more than welcome to! I will make a note
> that the function _could_ be refactored, but will work on other parts
> of the module during the upcoming days.
> 
> Thanks!
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Regards,
-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre


More information about the vlc-devel mailing list