RenderData does not write non-mandatory elements having a default value

[mbunkus]

Background

Let's take one of our new flags, KaxFlagHearingImpaired. It isn't mandatory, but it has a default value of 0.

Semantically there are three different states:

1. The element is not present, meaning no information is known about the question whether the track is suitable for hearing impaired users.
2. The element is present and set to 0: the track is NOT suitable for hearing impaired users.
3. The element is present and set to 1: the track IS suitable for hearing impaired users.

Bug

The various Render and RenderData functions in libebml contain a parameter governing whether or not libebml should write elements whose current value equals their default value. Unfortunately this function does NOT take into account whether or not the element is mandatory. They only compare the current value with the default value, both for mandatory and for non-mandatory elements.

So the situation is:

1. Element not present in data structure, no matter what bWithDefault is set to → element is not present in file. OK.
2. Element present in data structure, bWithDefault = true → element is present in file. OK.
3. Element present in data structure, bWithDefault = false → element is NOT present in file. Not OK, as the file representation is now semantically different than the representation in memory.

Proposed change

EbmlElement::Render() should take into account whether or not an element is mandatory. If it is not mandatory, its value should always be written, no matter what the bWithDefault parameter is set to.

How to reproduce

Here's an example program:

#include "ebml/EbmlHead.h"                                                                                                                                                                                                            
#include "ebml/EbmlSubHead.h"                                                                                                                                                                                                         
#include "ebml/StdIOCallback.h"                                                                                                                                                                                                       
#include "matroska/KaxSegment.h"                                                                                                                                                                                                      
#include "matroska/KaxSemantic.h"                                                                                                                                                                                                     
#include "matroska/KaxTracks.h"                                                                                                                                                                                                       
#include "matroska/KaxTrackEntryData.h"                                                                                                                                                                                               
                                                                                                                                                                                                                                      
using namespace libebml;                                                                                                                                                                                                              
using namespace libmatroska;                                                                                                                                                                                                          
                                                                                                                                                                                                                                      
int                                                                                                                                                                                                                                   
main(int,                                                                                                                                                                                                                             
     char **) {                                                                                                                                                                                                                       
  auto write_defaults = false;                                                                                                                                                                                                        
                                                                                                                                                                                                                                      
  StdIOCallback out{"test.mkv", MODE_CREATE};                                                                                                                                                                                         
                                                                                                                                                                                                                                      
  EbmlHead head;                                                                                                                                                                                                                      
  GetChild<EDocType>(head).SetValue("matroska");                                                                                                                                                                                      
  GetChild<EDocTypeVersion>(head).SetValue(4);                                                                                                                                                                                        
  GetChild<EDocTypeReadVersion>(head).SetValue(4);                                                                                                                                                                                    
  head.Render(out, write_defaults);                                                                                                                                                                                                   
                                                                                                                                                                                                                                      
  KaxSegment segment;                                                                                                                                                                                                                 
  auto &info   = GetChild<KaxInfo>(segment);                                                                                                                                                                                          
  auto &tracks = GetChild<KaxTracks>(segment);                                                                                                                                                                                        
                                                                                                                                                                                                                                      
  GetChild<KaxTimecodeScale>(info).SetValue(1000000);                                                                                                                                                                                 
  GetChild<KaxMuxingApp>(info).SetValue(L"dummy");                                                                                                                                                                                    
  GetChild<KaxWritingApp>(info).SetValue(L"dummy");                                                                                                                                                                                   
                                                                                                                                                                                                                                      
  tracks.PushElement(*new KaxTrackEntry);                                                                                                                                                                                             
  auto &track1 = dynamic_cast<EbmlMaster &>(*tracks[0]);                                                                                                                                                                              
                                                                                                                                                                                                                                      
  GetChild<KaxTrackNumber>(track1).SetValue(1);                                                                                                                                                                                       
  GetChild<KaxTrackUID>(track1).SetValue(1);                                                                                                                                                                                          
  GetChild<KaxTrackType>(track1).SetValue(track_video);                                                                                                                                                                               
  GetChild<KaxCodecID>(track1).SetValue("dummy1");                                                                                                                                                                                    
  GetChild<KaxFlagHearingImpaired>(track1).SetValue(1);                                                                                                                                                                               
                                                                                                                                                                                                                                      
  tracks.PushElement(*new KaxTrackEntry);                                                                                                                                                                                             
  auto &track2 = dynamic_cast<EbmlMaster &>(*tracks[1]);                                                                                                                                                                              
                                                                                                                                                                                                                                      
  GetChild<KaxTrackNumber>(track2).SetValue(2);                                                                                                                                                                                       
  GetChild<KaxTrackUID>(track2).SetValue(2);                                                                                                                                                                                          
  GetChild<KaxTrackType>(track2).SetValue(track_audio);                                                                                                                                                                               
  GetChild<KaxCodecID>(track2).SetValue("dummy2");                                                                                                                                                                                    
  GetChild<KaxFlagHearingImpaired>(track2).SetValue(0);                                                                                                                                                                               
                                                                                                                                                                                                                                      
  segment.WriteHead(out, 5, write_defaults);                                                                                                                                                                                          
                                                                                                                                                                                                                                      
  info.Render(out, write_defaults);                                                                                                                                                                                                   
  tracks.Render(out, write_defaults);                                                                                                                                                                                                 
                                                                                                                                                                                                                                      
  segment.ForceSize(out.getFilePointer() - segment.GetElementPosition() - segment.HeadSize());                                                                                                                                        
  segment.OverwriteHead(out);                                                                                                                                                                                                         
                                                                                                                                                                                                                                      
  return 0;                                                                                                                                                                                                                           
}

Compile with current libebml & libmatroska. Run it. Run mkvinfo test.mkv and observe:

[0 mosu@sweet-chili ~/test] ./matroska1 && mkvinfo test.mkv
+ EBML head
|+ Document type version: 4
|+ Document type read version: 4
+ Segment: size 70
|+ Segment information
| + Multiplexing application: dummy
| + Writing application: dummy
|+ Tracks
| + Track
|  + Track number: 1 (track ID for mkvmerge & mkvextract: 0)
|  + Track UID: 1
|  + Track type: video
|  + Codec ID: dummy1
|  + 'Hearing impaired' flag: 1
| + Track
|  + Track number: 2 (track ID for mkvmerge & mkvextract: 1)
|  + Track UID: 2
|  + Track type: audio
|  + Codec ID: dummy2

For both tracks the flag is explicitly set. However, libebml only writes the element if it is set to a value that is NOT its default value as is the case for track 1. For track 2 the element, even though it is present in the data structure to be written to disk, is not written as the value that was set equals the element's default value.

[robUx4]

To fix this there are (at least) two places to change: UpdateSize() and RenderData(). They both use if (!bWithDefault && Element->IsDefaultValue()) to determine if the element should be stored or not. That code is found in all base classes.

It should also check that it's mandatory with EbmlSemantic::IsMandatory().

[hubblec4]

To fix this there are (at least) two places to change: UpdateSize()

I also thought this method have to change, but now I'm not sure.

uint64 EbmlUInteger::UpdateSize(bool bWithDefault, bool /* bForceRender */)
{
if (!bWithDefault && IsDefaultValue())
return 0;

bWithDefault is set to false.
This method updates the size of the data and if a DefaultVaule is set and if IsDefaultValue() = true then we can omit the data,
and the value 0 for the size is fine.

For a non-mandatory element with a default value, the RenderData() method must write the element's head with a VINT size 0x80.

Therefore, I think UpdateSize() is not affected and maybe all other non-Master element type classes.

BUT for the Master element the UpdateSize() is wrong. It skips the element if the default value is used. But the header size of an element is not taken into account.

[hubblec4]

Proposed change
EbmlElement::Render() should take into account whether or not an element is mandatory. If it is not mandatory, its value should always be written, no matter what the bWithDefault parameter is set to.

Mmmh... then we lose the sense of the DefaultVaule attribute.
If the element is not mandatory the vaule should only be write if it's not the default vaule, otherwise we can skip the data for the vaule to save bytes.