[vlc-commits] mkv: remove typeid code in EBML dispatcher
Alexandre Janniaux
git at videolan.org
Wed Jun 17 17:55:53 CEST 2020
vlc | branch: master | Alexandre Janniaux <ajanni at videolabs.io> | Mon Jun 15 16:30:53 2020 +0200| [c764461180d70d1c9fa81e72cd7ad9d9b289eea6] | committer: Alexandre Janniaux
mkv: remove typeid code in EBML dispatcher
EBML can associate multiple class to a single EBML ID, which mean that
it potentially needs typeid checks. However, libmatroska always exposes
a single type per EBML ID, so it never needs those checks.
In addition, those checks are leading to warnings (attached below) and
issues depending on the visibility and optimization level on clang. See
the following mail on the mailing list for reference:
https://lists.llvm.org/pipermail/llvm-dev/2014-June/073465.html
To sum up, typeinfo are becoming different between libmatroska and the
matroska modules for the same classes, so the matroska demuxer is never
able to open correctly in those case, and it fallbacks on avformat
demuxer if available.
Compilation warning fixed by this patch:
../../../modules/demux/mkv/chapter_command.cpp:35:13: warning: expression with side effects will be evaluated despite being used as an operand to 'typeid'
[-Wpotentially-evaluated-expression]
if( MKV_CHECKED_PTR_DECL( p_cpt, KaxChapterProcessTime const, command[i] ) )
^
../../../modules/demux/mkv/mkv.hpp:116:63: note: expanded from macro 'MKV_CHECKED_PTR_DECL'
#define MKV_CHECKED_PTR_DECL( name, type, src ) type * name = MKV_IS_ID(src, type) ? static_cast<type*>(src) : NULL
^
../../../modules/demux/mkv/mkv.hpp:115:52: note: expanded from macro 'MKV_IS_ID'
#define MKV_IS_ID( el, C ) ( el != NULL && typeid( *el ) == typeid( C ) )
^
../../../modules/demux/mkv/chapter_command.cpp:44:13: warning: expression with side effects will be evaluated despite being used as an operand to 'typeid'
[-Wpotentially-evaluated-expression]
if( MKV_CHECKED_PTR_DECL( p_cpd, KaxChapterProcessData const, command[i] ) )
^
../../../modules/demux/mkv/mkv.hpp:116:63: note: expanded from macro 'MKV_CHECKED_PTR_DECL'
#define MKV_CHECKED_PTR_DECL( name, type, src ) type * name = MKV_IS_ID(src, type) ? static_cast<type*>(src) : NULL
^
../../../modules/demux/mkv/mkv.hpp:115:52: note: expanded from macro 'MKV_IS_ID'
#define MKV_IS_ID( el, C ) ( el != NULL && typeid( *el ) == typeid( C ) )
The issue was initially spotted through link-time warnings mentionning
incompatible visibility settings between the library archives and the
final static libvlc archive when compiling for iOS.
Fix videolan/VLCKit#372
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=c764461180d70d1c9fa81e72cd7ad9d9b289eea6
---
modules/demux/mkv/Ebml_dispatcher.hpp | 49 +++++------------------------------
modules/demux/mkv/chapter_command.cpp | 4 +--
modules/demux/mkv/mkv.hpp | 3 ++-
3 files changed, 11 insertions(+), 45 deletions(-)
diff --git a/modules/demux/mkv/Ebml_dispatcher.hpp b/modules/demux/mkv/Ebml_dispatcher.hpp
index 85bf03e609..8d1e7a6053 100644
--- a/modules/demux/mkv/Ebml_dispatcher.hpp
+++ b/modules/demux/mkv/Ebml_dispatcher.hpp
@@ -34,23 +34,14 @@
#include <vector>
namespace {
- namespace detail {
- template<class T>
- static std::type_info const* typeid_ptr () {
- static std::type_info const& ti = typeid (T);
- return &ti;
- }
- }
-
struct EbmlProcessorEntry {
typedef void (*EbmlProcessor) (EbmlElement*, void*);
EbmlId const* p_ebmlid;
- std::type_info const* p_typeid;
EbmlProcessor callback;
- EbmlProcessorEntry (EbmlId const& id, std::type_info const* ti, EbmlProcessor cb)
- : p_ebmlid (&id), p_typeid (ti), callback (cb)
+ EbmlProcessorEntry (EbmlId const& id, EbmlProcessor cb)
+ : p_ebmlid (&id), callback (cb)
{ }
};
@@ -84,7 +75,7 @@ namespace {
return false;
EbmlProcessorEntry eb = EbmlProcessorEntry (
- static_cast<EbmlId const&> (*element), NULL, NULL
+ static_cast<EbmlId const&> (*element), NULL
);
// --------------------------------------------------------------
@@ -98,26 +89,9 @@ namespace {
if (cit != cit_end)
{
- // --------------------------------------------------------------
- // normally we only need to compare the addresses of the EbmlId
- // since libebml returns a reference to a _static_ instance.
- // --------------------------------------------------------------
-
- while (cit != cit_end && (cit->p_ebmlid == eb.p_ebmlid || (*cit->p_ebmlid == *eb.p_ebmlid))) {
- std::type_info const& ti = typeid (*element);
-
- // --------------------------------------------------------------
- // even though the EbmlId are equivalent, we still need to make
- // sure that the typeid also matches.
- // --------------------------------------------------------------
-
- if (*(cit->p_typeid) == ti) {
- cit->callback (element, payload);
- return true;
- }
-
- ++cit;
- }
+ assert(cit->p_ebmlid == eb.p_ebmlid || (*cit->p_ebmlid == *eb.p_ebmlid));
+ cit->callback (element, payload);
+ return true;
}
if (_default_handler == NULL)
@@ -138,18 +112,9 @@ namespace {
InitializationExpr_, static_cast<EbmlType_&> (*data) \
)
-// -----------------------------------------------------------------------------------
-// The use of `detail::typeid_ptr` below is so that we do not have to invoke "typeid"
-// every time we are requested to do a lookup. `std::type_info` cannot be copied, so
-// we cannot pass it by value.
-//
-// In C++11 you could use the hash value present inside std::type_info to do lookup,
-// but we are stuck in C++03 and have to use the below instead.
-// -----------------------------------------------------------------------------------
-
#define E_CASE(EbmlType_, VariableName_) \
EBML_ELEMENT_CASE_DEF(EbmlType_, EbmlType_, VariableName_, \
- (dispatcher.insert( EbmlProcessorEntry( EbmlType_ ::ClassInfos.ClassId(), detail::typeid_ptr<EbmlType_>(), &EbmlType_ ## _callback) ) ) \
+ (dispatcher.insert( EbmlProcessorEntry( EbmlType_ ::ClassInfos.ClassId(), &EbmlType_ ## _callback) ) ) \
)
#define E_CASE_DEFAULT(VariableName_) \
diff --git a/modules/demux/mkv/chapter_command.cpp b/modules/demux/mkv/chapter_command.cpp
index 9a8749b879..9a23b77dfe 100644
--- a/modules/demux/mkv/chapter_command.cpp
+++ b/modules/demux/mkv/chapter_command.cpp
@@ -32,7 +32,7 @@ void chapter_codec_cmds_c::AddCommand( const KaxChapterProcessCommand & command
uint32 codec_time = uint32(-1);
for( size_t i = 0; i < command.ListSize(); i++ )
{
- if( MKV_CHECKED_PTR_DECL( p_cpt, KaxChapterProcessTime const, command[i] ) )
+ if( MKV_CHECKED_PTR_DECL_CONST( p_cpt, KaxChapterProcessTime, command[i] ) )
{
codec_time = static_cast<uint32>( *p_cpt );
break;
@@ -41,7 +41,7 @@ void chapter_codec_cmds_c::AddCommand( const KaxChapterProcessCommand & command
for( size_t i = 0; i < command.ListSize(); i++ )
{
- if( MKV_CHECKED_PTR_DECL( p_cpd, KaxChapterProcessData const, command[i] ) )
+ if( MKV_CHECKED_PTR_DECL_CONST( p_cpd, KaxChapterProcessData, command[i] ) )
{
std::vector<KaxChapterProcessData*> *containers[] = {
&during_cmds, /* codec_time = 0 */
diff --git a/modules/demux/mkv/mkv.hpp b/modules/demux/mkv/mkv.hpp
index 1e9a40e510..a061254228 100644
--- a/modules/demux/mkv/mkv.hpp
+++ b/modules/demux/mkv/mkv.hpp
@@ -112,8 +112,9 @@ enum
#define MKVD_TIMECODESCALE 1000000
-#define MKV_IS_ID( el, C ) ( el != NULL && typeid( *el ) == typeid( C ) )
+#define MKV_IS_ID( el, C ) ( el != NULL && (el->operator const EbmlId&()) == (C::ClassInfos.ClassId()) )
#define MKV_CHECKED_PTR_DECL( name, type, src ) type * name = MKV_IS_ID(src, type) ? static_cast<type*>(src) : NULL
+#define MKV_CHECKED_PTR_DECL_CONST( name, type, src ) const type * name = MKV_IS_ID(src, type) ? static_cast<const type*>(src) : NULL
using namespace LIBMATROSKA_NAMESPACE;
More information about the vlc-commits
mailing list