On Sat, 2011-10-01 at 19:22 +0200, Aleksander Morgado wrote:
> > I completed the Group 3 1D encoder and decoder. It shouldn't be
> > difficult (hope) now to create the 2D from that one, which I'll do next.
> > I send it in case someone wants to give it a try (and test it more, that
> > is something I'd like).
> >
> >
> > Regards,
> > Gustavo
> >
> > PS: It's base on Aleks' stream-api branch.
>
> Why is the COPYING file being removed in your patch?
> Why are there hash-related unit tests being included in the patch?
> Can you sync the patch with trunk and let it contain only your relevant
> changes? It will be much easier to review.
>
> As a general note, please try to follow the same coding style as all
> (most of) the other source files; I've spent a lot of time trying to
> give a common coding style lately...
>
> Specifically:
>
> Don't use tabs to indent, please; we do spaces only. Seems you end up
> mixing tab and spaces indentation in several places.
>
> Are these commented lines intended? If so, please add some comment
> explaining why and also use /* */ to comment the code. This also applies
> to multiple places in the patch.
> + union {
> + decoder_g31d_t *g31d;
> + //encoder_g32d_t *g32d;
> + //encoder_g4_t *g4;
> + } data;
>
> Please always include whitespace before the parenthesis in method
> definitions and calls, e.g. don't do:
> +encoder_find_span(pdf_bit_buffer_t *buf, int max_len, pdf_bool_t ones)
>
> Please follow the GCS for the use of parenthesis, alignment and such; so
> don't do:
> while (something) {
> foo;
> }
> Instead, it would be:
> while (something)
> {
> foo;
> }
>
> Instead of these debugging #ifdefs:
> +#ifdef G31DENCODER_DEBUG
> + fprintf(stderr, "encode_g31d_aplly: init\n");
> +#endif
> You can use PDF_DEBUG_BASE() and compile with --enable-debug-base. I
> think it will be much clearer.
>
> Give one arg per line in function definitions/declarations, and align
> them with spaces, no tabs. I usually also align arg names, although not
> that big deal. So, instead of:
> +encoder_put_bits(pdf_bit_buffer_t *buf, pdf_u16_t bits, int nbits)
> Better:
> +encoder_put_bits (pdf_bit_buffer_t *buf,
> pdf_u16_t bits,
> int nbits)
>
> I didn't really check what the patch does itself; got pain in the eye
> with so many coding style things :-).
>
> Could you update the patch with these comments, and we keep reviewing
> it? It really is much easier to review and maintain if all the code uses
> the same coding style.
>
> Thanks!
>
This is the patch to fix pdf-filter.c issue. Is it correct?
Regards,
Gustavo.
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: [email protected]
# target_branch: bzr://bzr.savannah.gnu.org/pdf/libgnupdf/trunk/
# testament_sha1: 9056b9107aecbf3564b3fb6acfb7805d6838f2ee
# timestamp: 2011-10-01 15:31:58 -0300
# base_revision_id: [email protected]
#
# Begin patch
=== modified file 'utils/pdf-filter.c'
--- utils/pdf-filter.c 2011-09-01 13:32:22 +0000
+++ utils/pdf-filter.c 2011-10-01 18:30:39 +0000
@@ -58,7 +58,7 @@
* Command line options management
*/
-enum
+enum filter_arg
{
FILTER_INSTALL_NONE,
HELP_ARG,
@@ -116,16 +116,20 @@
V2DEC_FILTER_INSTALL
};
-/* name filter args here */
-#define IS_FILTER_ARG(arg) \
- ((arg) == PRED_COLORS_ARG || \
- (arg) == PRED_BITSPERCOMPONENT_ARG || \
- (arg) == PRED_COLUMNS_ARG || \
- (arg) == PRED_PREDICTOR_ARG || \
- (arg) == JBIG2DEC_GLOBAL_SEGMENTS_ARG || \
- (arg) == JBIG2DEC_PAGE_SIZE || \
- (arg) == LZW_EARLYCHANGE_ARG || \
- (arg) == KEY_ARG)
+static pdf_bool_t
+is_filter_arg (enum filter_arg arg)
+{
+ return (arg == PRED_COLORS_ARG
+ || arg == PRED_BITSPERCOMPONENT_ARG
+ || arg == PRED_COLUMNS_ARG
+ || arg == PRED_PREDICTOR_ARG
+#ifdef HAVE_LIBJBIG2DEC
+ || arg == JBIG2DEC_GLOBAL_SEGMENTS_ARG
+ || arg == JBIG2DEC_PAGE_SIZE
+#endif /* HAVE_LIBJBIG2DEC */
+ || arg == LZW_EARLYCHANGE_ARG
+ || arg == KEY_ARG);
+}
static const struct option GNU_longOptions[] =
@@ -1314,7 +1318,7 @@
}
/* next arg is a new filter and current filter is with args */
- if (!IS_FILTER_ARG(next_ci) && filter_to_install != FILTER_INSTALL_NONE)
+ if (!is_filter_arg (next_ci) && filter_to_install != FILTER_INSTALL_NONE)
{
/* do extra loop installing current filter */
ci = filter_to_install;
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWSNg7iQAAc1fgBEQeXP//3//
37C//9/+UAQg2UmZsBHXOEkk0EnkZTMmg1HpMmEagnqaGEPQnqflT2ppPJ5QSiIMQ0NNCp4o0AAa
ABoyAAabSASRNJo0mianoyZBlNNDRoGmmgGgAAAI1FNH6poBoA9Q0ADQAAAAAADgGEYTTEMAgGQA
wjTJkwjAQ0gAwWcpu+DfjIqSrRrK2XHW6Etu2YBE8No98162fyhjzS58IChQb3/540UNXXD0fXAy
ZiTcHO/m1OL9UVMRCzM5128Bq7J9TghYOSUW7gbOHH8cMn2yf+2WG2AJXe3CRV144KU5kZi/cS12
YbLdfZHih87xIGAXUdguqbRlh7CbZhXfzYulbEryXoGuoom0zS3tsYFrMg51DlcIdQJ7M8zNCUtF
MgsHTXO5JODFcyuhQTcZsjSxCrez8/WqEgp57kYlKhM7N5wRpEuIpegEXPY06UG3KP9ZkN/ac8PD
SLqml8BIr0V6C2U1zgplmgjiiBQxupvpsfa4dcNwtMFmtVZCgoF7cWD2GjVLHVKuieS+KGoTNVH7
tr9xYZlO1NIwM0xNjJNvsGtnC6klWVlejB7MNOHA1M8K7VGYvoq5ynHFI5FeNRwGFrDJFUaTKKya
YsSrjrBKwilVtJ1aVOGaghMK2b0wxWEZaX7iyWWY56hTogwDTnxTILy+lTPdeUbXtXwozvkfLEnn
nBxE5M9nkYRRtP69tbfD1gbJqqKnMSAdO3X3JZe1b+Fv19Eu5xTODHjuCLcRrzgPs95otByKvTyk
u+IcyPv8Ipz5fRvFueiYg7ljhL5OOYx2hzOZkpUjSEdkVvqsqZVvqShzs8B96Y5tzdg5kQfWhYBa
NpHQW9kLLbqBsDV6zekmua7Ttr94ooWym8taPHye5cnDLjcI44Oi2Qf7l5z3/HbLLcCBLT87s7Qx
fSNXEWhnRuYcz1IwDa/jIqPl7v5x45sHFfqKrw+TDd88/9d/h02WKWeR0yU8HRGxP0TEkKeqzGve
P+cSauV9oYBOcTpRxYvAXWYugmyHJ2ht/ioDV15L2FnXZA19OiPNZZO2/Tlm672ZsX3cQ/8vxzmx
Mp9ty+r2JzmFq0Z8pzIwRKB5RJizZqtRBDTJotHDzjV6tMQ7tTF9jtGpBHaj/TcNDLySKFeFiwoy
BHgvofiiM5O5uenAmfxu9TFKWFQsgYw6dIUS566XyVBcg4dYerP5Cj15SVcsLfNOUJb9vVScJBJ8
BlWa0cjGXmoKYi2RbaLupNtulJa1AFctxmiINPYVKWJmK0vgHrPVGyyUpoZlgDDrgrvZWAcu4IL9
1Lo1pPVHLu1Lw5gejFZWDxlUGMpCA0lpLcjXkODUuQlkEAGRMLBhAIzZSaiBKgW7HV+jDqTedjQy
x6FLZssyLCtAai4mN5qaFlhBCaWQk5fKxS8OojJySKiZiyfNdMMB9AEbgiFNd6NSEmBrOQi7qBXQ
l9NtYj0ckA4TYJjaTtTDnkCCfuYuCrFjpF7hXXFyzzS5JO/Iu5IpwoSBGwdxIA==
