write_nal_unit() function prints spare null byte at buf[0]

[ghost]

 $ cat ./build.sh 
#!/bin/bash                                                                                                                                                                                   

gcc -I /usr/local/src/h264bitstream/   gen_stream.c   /usr/local/src/h264bitstream/.libs/libh264bitstream.a   -o gen_stream
[OK]
15:49:26krieger@zver ~/work/employers/bluecherry/h264_stream_gen
 $ ./build.sh 
[OK]
15:49:27krieger@zver ~/work/employers/bluecherry/h264_stream_gen
 $ ./gen_stream 
Stream start headers:
00 00 00 01 00 67 42 00 1e 92 44 05 a0 92 20 00 00 00 01 00 68 ce 08 48 80 
Header before keyframe:
00 00 00 01 00 25 b8 04 00 
Header before non-key frame 1:
00 00 00 01 00 21 e0 41 
Header before non-key frame 2:
00 00 00 01 00 21 e0 82 
Header before non-key frame 3:
00 00 00 01 00 21 e0 c3 
[OK]

#include <h264_stream.h>

#define MIN(x, y) ({                            \
        typeof(x) _min1 = (x);                  \
        typeof(y) _min2 = (y);                  \
        (void) (&_min1 == &_min2);              \
        _min1 < _min2 ? _min1 : _min2; })

h264_stream_t* h = NULL;
uint8_t marker[] = {0x00, 0x00, 0x00, 0x01};

ssize_t write_stream_start(uint8_t *buf, size_t size)
{
        ssize_t off = 0;

        /* SPS */
        if (size - off - 4 < 0)
                return off;  // TODO indicate error and not just "normal" premature return
        memcpy(buf + off, marker, MIN(sizeof(marker), size - off));
        off += 4;

        h->nal->nal_ref_idc = 0x03;
        h->nal->nal_unit_type = NAL_UNIT_TYPE_SPS;

        h->sps->profile_idc = 0x42;  /* == 66, baseline */
        h->sps->level_idc = 30;
        h->sps->log2_max_frame_num_minus4 = 3;
        h->sps->log2_max_pic_order_cnt_lsb_minus4 = 3;
        h->sps->num_ref_frames = 0x01;
        h->sps->pic_width_in_mbs_minus1 = 0x2c;  /* == 44 */
        h->sps->pic_height_in_map_units_minus1 = 35; // 0x1d;
        h->sps->frame_mbs_only_flag = 0x01;

        off += write_nal_unit(h, buf + off, size - off);

        /* PPS */
        if (size - off - 4 < 0)
                return off;  // TODO indicate error and not just "normal" premature return
        memcpy(buf + off, marker, MIN(sizeof(marker), size - off));
        off += 4;

        h->nal->nal_ref_idc = 0x03;
        h->nal->nal_unit_type = NAL_UNIT_TYPE_PPS;

        h->pps->pic_parameter_set_id = 0;
        h->pps->seq_parameter_set_id = 0;
        h->pps->entropy_coding_mode_flag = 0;
        h->pps->pic_order_present_flag = 0;
        h->pps->num_slice_groups_minus1 = 0;
        h->pps->num_ref_idx_l0_active_minus1 = 0;
        h->pps->num_ref_idx_l1_active_minus1 = 0;
        h->pps->weighted_pred_flag = 0;
        h->pps->weighted_bipred_idc = 0;
        h->pps->pic_init_qp_minus26 = 2;
        h->pps->pic_init_qs_minus26 = 2;
        h->pps->chroma_qp_index_offset = 0;
        h->pps->deblocking_filter_control_present_flag = 0;
        h->pps->constrained_intra_pred_flag = 0;
        h->pps->redundant_pic_cnt_present_flag = 0;

        off += write_nal_unit(h, buf + off, size - off);

        return off;
}

ssize_t write_stream_middle_idr(uint8_t *buf, size_t size)
{
        ssize_t off = 0;

        if (size - off - 4 < 0)
                return off;  // TODO indicate error and not just "normal" premature return
        memcpy(buf + off, marker, MIN(sizeof(marker), size - off));
        off += 4;

        h->nal->nal_ref_idc = 1;
        h->nal->nal_unit_type = NAL_UNIT_TYPE_CODED_SLICE_IDR;
        h->sh->first_mb_in_slice = 0;
        h->sh->slice_type = 2;
        h->sh->pic_parameter_set_id = 0;
        h->sh->frame_num = 0;
        h->sh->idr_pic_id = 1;
        h->sh->pic_order_cnt_lsb = 0;
        h->sh->drpm.no_output_of_prior_pics_flag = 0;
        h->sh->drpm.long_term_reference_flag = 0;
        h->sh->slice_qp_delta = 0;

        off += write_nal_unit(h, buf + off, size - off);

        return off;
}

ssize_t write_stream_middle_non_idr(uint8_t *buf, size_t size, int frame_num_in_gop)
{
        ssize_t off = 0;

        if (size - off - 4 < 0)
                return off;  // TODO indicate error and not just "normal" premature return
        memcpy(buf + off, marker, MIN(sizeof(marker), size - off));
        off += 4;

        h->nal->nal_ref_idc = 1;
        h->nal->nal_unit_type = NAL_UNIT_TYPE_CODED_SLICE_NON_IDR;
        h->sh->first_mb_in_slice = 0;
        h->sh->slice_type = SH_SLICE_TYPE_P;
        h->sh->pic_parameter_set_id = 0;
        h->sh->frame_num = frame_num_in_gop;
        h->sh->idr_pic_id = 1;
        h->sh->pic_order_cnt_lsb = frame_num_in_gop * 2;  /* for interlaced */
        h->sh->drpm.no_output_of_prior_pics_flag = 0;
        h->sh->drpm.long_term_reference_flag = 0;
        h->sh->slice_qp_delta = 0;

        off += write_nal_unit(h, buf + off, size - off);

        return off;
}

void print_buf(uint8_t *buf, ssize_t size)
{
        ssize_t i;
        for (i = 0; i < size; i++) {
                printf("%02x ", buf[i]);
        }
        printf("\n");
}

int main(int argc, uint8_t *argv[])
{
        ssize_t stream_start_len = 0;
        ssize_t stream_middle_len = 0;
        uint8_t stream_start[64] = { 0, };
        uint8_t stream_middle[64] = { 0, };
        int i;

        h = h264_new();

        printf("Stream start headers:\n");
        stream_start_len = write_stream_start(stream_start, sizeof(stream_start));
        print_buf(stream_start, stream_start_len);

        printf("Header before keyframe:\n");
        stream_middle_len = write_stream_middle_idr(stream_middle, sizeof(stream_middle));
        print_buf(stream_middle, stream_middle_len);

        for (i = 1; i < 4; i++) {
                printf("Header before non-key frame %d:\n", i);
                stream_middle_len = write_stream_middle_non_idr(stream_middle, sizeof(stream_middle), i);
                print_buf(stream_middle, stream_middle_len);
        }

        return 0;
}

[antoneliasson]

This is probably a side effect of rbsp_to_nal() starting the NAL buffer with a zero byte. I am interpreting the reason for this behaviour to be that the first byte in the resulting NAL is reserved for the NAL header. But write_nal_unit() puts the NAL header first in the RBSP stream before passing it to rbsp_to_nal(), causing the extra zero byte.

If this is the intended behaviour of rbsp_to_nal(), lines 1059-1062 in h264_stream.c which currently do the following:

    bs_t* b = bs_new(rbsp_buf, rbsp_size);
    /* forbidden_zero_bit */ bs_write_u(b, 1, 0);
    bs_write_u(b, 2, nal->nal_ref_idc);
    bs_write_u(b, 5, nal->nal_unit_type);

should be changed to instead modify the first byte of the NAL unit resulting from the (later) call to rbsp_to_nal().

[skirsten]

@antoneliasson Thank you very much. I manually changed the code as you suggested and it fixed my problem.

[antoneliasson]

Good. Could you please submit a pull request for your change?

[aizvorski]

@antoneliasson @skirsten This sounds like a serious bug, but I'm not sure I completely understand what the bug is supposed to be.

Per the standard, nals may start with either 0x00 00 00 01 or 0x00 00 01. When writing, h264bitstream always uses the longer form (regardless of what it was when that nal was read in - if it was read in). This is consistent with eg x264 which always writes the longer form.

[antoneliasson]

You are talking about the start code prefix, which can be either 0x00 00 01 or 0x00 00 00 01 (for 16-bit alignment purposes if I remember correctly). The start code prefix must be immediately followed by the NAL Unit header. The bug is that your application inserts a zero byte between the start code prefix and the NAL Unit header.

[skirsten]

I wanted to push the changes originally but I deleted the code and forgot about it. Will do now... Edit: Nevermind I have no clue what I changed...

[mansourmoufid]

I stumbled on this too...

I was expecting:

SPS: 00 00 00 01 67 42 00 0a f8 41 a2 
PPS: 00 00 00 01 68 ce 38 80 

but kept getting:

SPS: 00 67 42 00 0a f8 41 a2 
PPS: 00 68 ce 38 80 

Anyway, I worked around it by just writing 3 bytes ahead in the buffer then overwriting the first four bytes with the start code. I.e. replace write_nal_unit(stream, buffer, size) with (write_nal_unit(stream, buffer + 3, size - 3) + 3).

[mansourmoufid]

I guess there was a confusion between "NAL unit syntax" (7.3.1) and "byte stream NAL unit syntax" (B.1.1). The latter begins with "leading_zero_8bits":