Working branch by MLsmd · Pull Request #34 · kit-cel/gr-dab

MLsmd · 2017-12-18T15:57:09Z

Improvements, additions and bug fixes for DABstep
New Synchronization approach and OFDM Demodulation
Remove all elements from previous Synchronization

…le services

git-subtree-dir: fdk-aac-dab git-subtree-split: 89639e36b29a622c641c3de3a4737a4c848dc365

grc: fixed category naming

add a std::runtime_error if aac enc init fails.

imported dab_swig as dab in tests

…espectively

MLsmd · 2018-01-18T16:13:04Z

Thanks for the review.!
I did the requested changes and some more wrap up, docu, ...
I think we are ready for merge in master now.

…to working_branch

marcusmueller · 2018-02-23T13:53:53Z

lib/fib_source_b_impl.cc

-                                                         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
-    // 000 00110 000 00000 0100000000000000 00 0 0000000000000 00000000
+        //ensemble info, CIF counter has to increase with each CIF
+        const char fib_source_b_impl::d_ensemble_info[56] = {0, 0, 0, 0, 0, 1,


Do the line breaks have a "semantic" here? the array "formatting" doesn't seem to reflect the spaces in the binary string comment above

This is caused to autoformat. I think we can cross the 80 column border in this case.

marcusmueller · 2018-02-23T13:54:31Z

lib/fib_sink_vb_impl.cc

            case FIB_SI_EXTENSION_DATA_SERVICE_LABEL:
              memcpy(label, &data[5], 16);
-              GR_LOG_DEBUG(d_logger, format("[data service label]: %s") % label);
+              GR_LOG_DEBUG(d_logger,


My happiness about this kind of line breaks is limited, but it's up to your coding style, for sure.

marcusmueller

This is outstanding in quality! Thank you, Luca!

So, the only things I think need changing are:

you forgot to fix python/GUI/CMakeLists
you actually reformatted third-party code, and should revert that. Same goes for modtool-generated test_dab.cc
you used an autoformatter that very strictly breaks, but not very cleverly, which actually hurts readability. Not quite sure I want you to spend time on improving that - seemingly, you didn't do that in an isolated commit (....).

Fix 1 and 2, and we have a merge. Come up with a solution for 3, and I'd be even more happy than now!

marcusmueller · 2018-02-23T13:56:05Z

lib/fib_source_b_impl.cc

+          }
+          if (data_rate_n.size() != num_subch) {
+            throw std::invalid_argument((boost::format(
+                    "size of vector data_rate_n (%d) does not fit with number of subchannels (%d)") %


the way you added line breaks here isn't really improving readability of the statement.

marcusmueller · 2018-02-23T13:57:00Z

lib/fib_source_b_impl.cc

+            throw std::invalid_argument((boost::format(
+                    "size of service label strings is (%d) but should be %d * 16 = %d") %
+                                         programme_service_labels.size() %
+                                         num_subch % (16 * num_subch)).str());


if you do it like this, why not directly go for one argument per line? Better than to just mechanically break at N characters.

marcusmueller · 2018-02-23T13:59:33Z

lib/fib_source_b_impl.cc

+              //increase CIF counter
+              if (d_transmission_mode != 3)
+                CIF_counter(out + d_offset, d_nFIBs_written / 3);
+              else CIF_counter(out + d_offset, d_nFIBs_written / 4);


statement1; else statement2; statement3;

is no good. I'd generally advertise usage of {}, even on single-line blocks, but more importantly, never do the "inline else"; on a scan, it looks like statement3 is only executed conditionally.

marcusmueller · 2018-02-23T14:00:08Z

lib/fib_source_b_impl.cc

+                }
+              }
+              d_offset += 8;
+              while (d_offset % (8 * FIB_LENGTH) !=


Um, no, there shouldn't be a line break before 0).

marcusmueller · 2018-02-23T14:00:58Z

lib/fib_source_b_impl.cc

+                           5);
+              //subchannel orga field
+              for (int subch_count = 0; subch_count <
+                                        d_num_subch; subch_count++) {//iterate over all subchannels


um, no, when adding a line break (if at all) in a for, do it at the ;, not in the middle of a comparison.

marcusmueller · 2018-02-23T14:59:04Z

lib/sum_phasor_trig_vcc_impl.cc

+              out[j] = in[j] * d_last_symbol[j];
+          } else {
+            for (unsigned int j = 0; j < d_length; j++) {
+              // printf("%p\n", &lastout[j]);


remove printf comments.

marcusmueller · 2018-02-23T14:59:48Z

lib/test_dab.cc

  CppUnit::TextTestRunner runner;
  std::ofstream xmlfile(get_unittest_path("dab.xml").c_str());
-  CppUnit::XmlOutputter *xmlout = new CppUnit::XmlOutputter(&runner.result(), xmlfile);
+  CppUnit::XmlOutputter *xmlout = new CppUnit::XmlOutputter(&runner.result(),


this is an autogenerated file. Why did you reformat it?

marcusmueller · 2018-02-23T15:00:29Z

lib/time_interleave_bb_impl.cc

          //*out++ = in[vec_length * (i + (scrambling_length-1) - d_scrambling_vector[j % scrambling_length]) + j];
-          *out++ = in[d_vector_length * (i + (d_scrambling_length - 1) - d_scrambling_vector[j % d_scrambling_length]) +
+          *out++ = in[d_vector_length * (i + (d_scrambling_length - 1) -
+                                         d_scrambling_vector[j %


um, no, not really the place to break this line.

marcusmueller · 2018-02-23T15:02:15Z

python/GUI/CMakeLists.txt

    usrp_dab_rx.py
    usrp_dab_tx.py
-    user_frontend.py
+    ${GUI_PY}


I'm under the impression that you forgot to move the set(GUI_PY… lines above this line, so that this works?

marcusmueller · 2018-02-23T15:03:09Z

python/__init__.py

-	# this might fail if the module is python-only
-	from dab_swig import *
+    # this might fail if the module is python-only
+    from dab_swig import *


Well, if you touch these lines, you might as well remove the try/except ImportError: pass, as this module is definitily not python-only.

marcusmueller · 2018-02-26T14:10:43Z

So, formally OK, now we just have to fix the build failures:

../lib/fib_sink_vb_impl.cc:84:23: error: return type of out-of-line definition of 'gr::dab::fib_sink_vb_impl::process_fig' differs from that in the declaration
    fib_sink_vb_impl::process_fig(uint8_t type, const char *data, uint8_t length) {
                      ^
../lib/fib_sink_vb_impl.h:51:11: note: previous declaration is here
      int process_fig(uint8_t type, const char *data, uint8_t length);
      ~~~ ^

I think you meant to use void here, as the actual implementation never returns a value?

marcusmueller · 2018-02-26T14:11:25Z

(also, hint, maintainers love code that is going to merge smoothly into master, so a git rebase -s ours might be a good idea)

MLsmd · 2018-02-26T15:17:33Z

I am not yet finished with reformatting and documenting the whole code. I will give you a hint here when I finished the work.
And thanks for the tip with rebase -s ours, I am going to try it out :)

MLsmd · 2018-03-06T18:39:57Z

I am now finally finished with reformatting and documenting the rest of the code and ready for the merge.

marcusmueller · 2018-03-06T18:42:56Z

Juche! Will review tomorrow :)

relative_rate: cast integer values to float before division

Fix the memory allocation problem and add ofdm_sync & diff_phasor speed improvements

MLsmd and others added 30 commits August 11, 2017 16:12

GUI: add GNU radio plots in developer mode

9998c6a

GUI window size

984565f

GUI: logo, superframe display flow(devMode)

e5d66b6

GUI: add DAB mode (next to DAB+)

7d521bc

GUI: add case: no MCI in FIC, prevent from crashing whole programm

e39cc9f

GUI: bug fix JSON

3b735bd

auto adjust sample rate + programme type support

f5a7396

FIB_source: change structure to one subchannel per service and multip…

260543c

…le services

transmission mux bug fix

5ae9b92

FIB_source: add individual labels for each service

87641b0

GUI transmitter: add dab/dab+ selection for each service individually

3f6896c

add a std::runtime_error if aac enc init fails.

f696245

add another throw statement

c34c68c

GUI: enable physical audio source (e.g. microphone) as msc audio source

0b5e672

grc: fixed category naming

d96cec3

Squashed 'fdk-aac-dab/' content from commit 89639e3

9cb9134

git-subtree-dir: fdk-aac-dab git-subtree-split: 89639e36b29a622c641c3de3a4737a4c848dc365

Merge commit '9cb9134bce0e3b0b0a83b23676c3ba2bf6c6655d' as 'fdk-aac-dab'

407497d

added renaming to bootstrap

065978d

Included fdk_aac_dab

0e26c87

renamed fdk-aac to fdk-aac-dab

6cc35df

Documentation on installing fdk-aac-dab: Readme and useful CMake output

8695426

imported dab_swig as dab in tests

5f37af8

GUI: prevent overflow of labels and set border for maximum of services

2bbe536

receiver: fix fatal bug in mp2 decoding

328c1b0

Merge pull request #18 from kit-cel/fix_grc_category

ad0e314

grc: fixed category naming

Merge pull request #17 from kit-cel/exit_on_error

0d9a43c

add a std::runtime_error if aac enc init fails.

Merge pull request #19 from kit-cel/fix_tests

2af41ce

imported dab_swig as dab in tests

removed header that becomes redundant through fdk-aac-dab

507bcf6

GUI: merge DAB and DAB+ transmitter/receiver flowgraph in one class r…

0dd004e

…espectively

Merge branch 'developer_front_end' into working_branch

2be5dbb

MLsmd added 2 commits January 18, 2018 16:53

wrap up, rm redundancy, add docu

e80bacd

fix error in qa test

7f59a1c

MLsmd and others added 6 commits January 21, 2018 22:46

remove unused stuff

61320c4

fix except status in python module init

7a47098

generate user_frontend.py on the fly while building

3312591

Merge branch 'working_branch' of https://github.com/kit-cel/gr-dab in…

4bc3c43

…to working_branch

PyQt4 pyuic integration fix

f117e0b

GUI: suppress the SNR update info, closes #29

fa166fd

marcusmueller reviewed Feb 23, 2018

View reviewed changes

marcusmueller requested changes Feb 23, 2018

View reviewed changes

MLsmd added 3 commits February 23, 2018 20:44

GUI: correct CMake installation order in python/GUI

cea6fc7

reformat auto-formatted garbage

1e78124

reformatting and docu

b43c759

marcusmueller approved these changes Feb 26, 2018

View reviewed changes

reformatting and documentation

4a343fd

noc0lour and others added 8 commits August 14, 2018 17:00

relative_rate: cast integer values to float before division

5ac53ad

relative_rate: cast integer values to float before division

3ae983a

ofdm_sync: speed improvements through volkizing

fa9eb52

diff_phasor: volkize

8ec05f4

fib_sink: replace assert, comment not working solution

8ab7ed7

mp4_decode: replace throw with debug statement

2b8efb6

Merge pull request #41 from noc0lour/fix_relative_rate

e842a62

relative_rate: cast integer values to float before division

Merge pull request #42 from noc0lour/speed_improvements

8a02e81

Fix the memory allocation problem and add ofdm_sync & diff_phasor speed improvements

Conversation

MLsmd commented Dec 18, 2017

Uh oh!

MLsmd commented Jan 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcusmueller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcusmueller commented Feb 26, 2018

Uh oh!

marcusmueller commented Feb 26, 2018

Uh oh!

MLsmd commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MLsmd commented Mar 6, 2018

Uh oh!

marcusmueller commented Mar 6, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MLsmd commented Feb 26, 2018 •

edited

Loading