MPEGGroup / FileFormat

MPEG file format discussions
23 stars 0 forks source link

handler_type in SampleDescriptionBox (8.5.2.2) and SampleGroupDescriptionBox (8.9.3.2) #28

Closed lkondrad closed 3 years ago

lkondrad commented 3 years ago

Based on WG03N0148 (ISOBMFF 7th edition)

SampleDescriptionBox (8.5.2.2) and SampleGroupDescriptionBox (8.9.3.2) have handler_type as input to the class, but the handler_type is not store anywhere and there is no semantics for it.

Is it a bug or is handler_type intentional there? If the latter then it would be probably good to have a clarification text.

aligned(8) class SampleDescriptionBox (unsigned int(32) handler_type) extends FullBox('stsd', version, 0){   int i ;   unsigned int(32) entry_count;   for (i = 1 ; i <= entry_count ; i++){     SampleEntry(); // an instance of a class derived from SampleEntry   } }

aligned(8) class SampleGroupDescriptionBox (unsigned int(32) handler_type) extends FullBox('sgpd', version, 0){   unsigned int(32) grouping_type;   if (version>=1) { unsigned int(32) default_length; }   if (version>=2) {     unsigned int(32) default_group_description_index;   }   unsigned int(32) entry_count;   int i;   for (i = 1 ; i <= entry_count ; i++){     if (version>=1) {       if (default_length==0) {         unsigned int(32) description_length;       }     }     SampleGroupDescriptionEntry (grouping_type);     // an instance of a class derived from SampleGroupDescriptionEntry     // that is appropriate and permitted for the media type   } }

cconcolato commented 3 years ago

Previous editions of ISOBMFF (e.g. 2012, 4th edition) used to have a switch on the handler to select a type of SampleEntry:

aligned(8) class SampleDescriptionBox (unsigned int(32) handler_type) extends FullBox('stsd', 0, 0){ 
  int i ; 
  unsigned int(32) entry_count; 
  for (i = 1 ; i <= entry_count ; i++){ 
    switch (handler_type){ 
    case ‘soun’: // for audio tracks 
      AudioSampleEntry(); 
      break;
    case ‘vide’: // for video tracks 
      VisualSampleEntry(); 
      break; 
    case ‘hint’: // Hint track 
      HintSampleEntry(); 
      break; 
    case ‘meta’: // Metadata track 
      MetadataSampleEntry(); 
      break;
    } 
  } 
 } 

I think this is left over and it could be removed.

cconcolato commented 3 years ago

Note that this is a duplicate of https://github.com/MPEGGroup/FileFormat/issues/21

dwsinger commented 3 years ago

Ficed in the 7th edition, closing